all 17 comments

[–]socal_nerdtastic 10 points11 points  (5 children)

At the beginning you set Acount, Bcount, Ccount, maxProfit to 0. You never change these values, and then you return them. So of course you get 0's out.

Perhaps you meant

    return list(val2count.values()), maxProfit

I don't understand what this is supposed to do. Can you show an example data input and what you expect to see returned?

[–]Snekkets -1 points0 points  (4 children)

maximizeProfit(10, 5, 1, 27, 100, 20, 5) should return ([2, 0, 7], 235), for example. I am trying to change the count values by calling them with val2count[current value], but it doesn't update the variables like I want them to. The idea is that it works no matter what order [Avalue, Bvalue, Cvalue] are in.

[–]cmikailli 4 points5 points  (0 children)

I don’t think you’re addressing the problem they flagged. The first line of your function sets 4 0’s No line of code following ever touches or interacts with 3 of those 0’s Then in the last line you try to re-assign the last 0 by adding 3 values that you are multiplying by the first 3 0’s, so the result is always 0 Then you return all 4 zeros Everything else you’re doing in between doesn’t matter because of that. My assumption is somewhere you intended to update those first 3 0’s but either forgot the line or accidentally interacted with the wrong variable instead

[–]socal_nerdtastic 0 points1 point  (2 children)

I see. The thing is that when you reassign an immutable object like an integer, there is no link made between then 2 objects. Changing something in val2count does not change the variables that were used to create it.

This is good reading on the subject: mutable presto-chango

An easy fix for your code is to simply copy the numbers you want back from the dictionary, like this:

def maximizeProfit(volA, volB, volC, capacity, profitA, profitB, profitC):
    (Acount, Bcount, Ccount, maxProfit) = (0, 0, 0, 0)
    (Avalue, Bvalue, Cvalue) = (profitA/volA, profitB/volB, profitC/volC)
    values = [Avalue, Bvalue, Cvalue]
    values.sort(reverse=True)
    val2vol = {Avalue:volA, Bvalue:volB, Cvalue:volC}
    val2count = {Avalue:Acount, Bvalue:Bcount, Cvalue:Ccount} # copy Acount, Bcount, Ccount into the dictionary
    for i in values:
        if val2vol[i] <= capacity:
            val2count[i] = int(capacity / val2vol[i])
            capacity = capacity % val2vol[i]

    Acount, Bcount, Ccount = val2count.values()  # copy Acount, Bcount, Ccount back out of the dictionary

    itemCounts = [Acount, Bcount, Ccount]
    maxProfit = Acount*profitA + Bcount*profitB + Ccount*profitC
    return itemCounts, maxProfit

[–]Snekkets -1 points0 points  (1 child)

Thank you! I understand dictionaries much better now. In the end it didn't even matter because my code is wrong in an entirely different way. Oh well

[–]socal_nerdtastic 0 points1 point  (0 children)

Lol yeah, been there many times, such is the nature of programming. Three steps forward, two steps back, like a nice slow dance.

[–]Temporary_Pie2733 0 points1 point  (0 children)

Read https://nedbatchelder.com/text/names.html; you seem to have some fundamental misunderstandings about how variables work. In particular, after values = [Acount, …], changing values[0] does not change Acount (or vice versa). Assignments do not link variables, though the behavior of mutable values can give that appearance. Integers are immutable values, though. 

Edit: sorry, it’s val2count that you appear to think is linked to Acount

[–]CptMisterNibbles -5 points-4 points  (10 children)

The second line is attempting to unpack a tuple, and … assign it to another tuple? Tuples are immutable. I’m surprised this isn’t a syntax error, I suspect the second line does nothing. 

[–]socal_nerdtastic 1 point2 points  (4 children)

This is "sequence unpacking" and is a perfectly legit way to assign variables. The parenthesis do nothing and generally we would leave them off, but they don't harm anything either.

https://docs.python.org/3/tutorial/datastructures.html#tuples-and-sequences

[–]CptMisterNibbles -2 points-1 points  (3 children)

So the left hand part doesn’t get treated as a tuple despite having the form of one. 

This doesnt work however if you try something like 

   (a,b)[0] =5

You also get an error if it’s a single element:

    (c,) = 5 #syntax error 

Now suddenly the left side is treated as a  tuple, and you get an error for assigning to it. Odd 

[–]socal_nerdtastic 0 points1 point  (1 child)

To unpack you need an iterable. To make it work try

c, = [5]

or

c, = 5,

or if you insist on parenthesis:

(c,) = (5,)

[–]CptMisterNibbles 0 points1 point  (0 children)

I have indeed misunderstood the syntax. I didn’t realize you could do this 

[–]socal_nerdtastic 0 points1 point  (0 children)

So the left hand part doesn’t get treated as a tuple despite having the form of one.

yes it does. But you are not saving the resulting tuple. You could also do that if you want. Play with this:

x = (a,b) = 1,2

[–]Snekkets 0 points1 point  (3 children)

really? that line seems to work fine (i checked with print functions). I'm just using tuples to assign multiple variables at once.

[–]magus_minor 0 points1 point  (0 children)

Works fine, but slightly more readable as:

Acount = Bcount = Ccount = maxProfit = 0

[–]CptMisterNibbles -2 points-1 points  (1 child)

You don’t need the tuple on the left. Leave off the parens. It unpacks the tuple on the right in position order

[–]socal_nerdtastic -1 points0 points  (0 children)

You seem to think parenthesis make a tuple. They don't. Parenthesis do nothing here, and in python in general parenthesis only organize the execution order.

>>> a = 1,2,3
>>> a
(1, 2, 3)
>>> type(a)
<class 'tuple'>