Suggestions to improve a code

[jmp]

On 09/06/2016 04:55 PM, GP wrote:
> I have a list:
> shelves2 =[{'Part No': '1', 'Length': 610.0,  'Width': 50.0},
>         {'Part No': '2', 'Length': 2319.0, 'Width': 465.0 },
>         {'Part No': '3', 'Length': 5.0,'Width': 465.0}]
>
> The length of shelf is calculated as follows:
>   1. Calculate the maximum length of all items in the shelf.
>   2. Calculate width of shelf = 610 (in this case).
>   3. Length of shelf : maxlength of all items + length ( if it less than width of shelf) or width of other items such that length of shelf  is less than maximum allowed length.
>
> In the above case the length is 2319+50+5 = 2374. I have the following code which generates it correctly but would like to know if there is a better way to write it. I would like to know some smarter way to write the code ( especially if the "for", "if" statements can be avoided).
>    list_1=[]
>    list_2=[]
>    WidthOfShelf = max([x['Width'] for x in shelves2])
>    MaxLengthOfItem =  max([x['Length'] for x in shelves2])
>    f_LMax=lambda seq, m: [ii for ii in range(0, len(seq)) if seq[ii] ['Length'] >= m][0]
>    MaxLengthOfItem_Index =f_LMax(shelves2,MaxLengthOfItem)
>    current= MaxLengthOfItem
>
>    list_1=[shelves2[MaxLengthOfItem_Index]]
>    list_2 = [MaxLengthOfItem]
>    del shelves2[MaxLengthOfItem_Index]
>    for i in range(0,len(shelves2)):
>         if shelves2[i]['Length'] <= Width:
>             if (current + shelves2[i]['Length']) or (current + shelves2  [i]['Width'])<= 2438.5 :
>                 q_2= min(shelves2[i]['Length'],  shelves2[i]['Width'])
>                 q=shelves2[i]
>                 list_1.append(q)
>                 list_2.append(q_2)
>                 current += (shelves2[i]['Length'] or shelves2[i]  ['Width'])
>                 length = sum(list_2)
>    print("LENGTH:",length)
>


Hi,

Your code does not make any sense. Before making it smarter, you need to 
make it understandable if you want us to help you.

It's ok to write quick and dirty scripts, until you send them for others 
to review :)

Here are my advices:

1/ use meaningful names
   * list_1, list_2, q_2, q, current give no clue about what information 
they're supposed to hold
   * you can use comments instead as sometimes a short name won't do it

2/ provide code that reproduces your problem, executing the above code 
gives a NameError.
---> 30      if shelves2[i]['Length'] <= Width:
NameError: name 'Width' is not defined

3/ name different things with different names. at the end of your code 
you're printing "length", which is the sum of either length or width fields.
It's difficult to guess what you're trying to achieve. Same confusion 
about the field 'Length' of a shelf, and the length of a shelf that is 
computed elsewhere. My guess would be that a shelf has an innate length, 
and another one depending on the  shelves it's inserted into. If so, 
find 2 different names.

4/ You are iterating like a C/C++ addict. Short story: resist the urge 
to iterate over indexes.

for i in range(0,len(shelves2)):
     print(shelves2[i]['Length'])

is best written

for shelf in shelves2:
    print(shelf['Length'])

long story : http://nedbatchelder.com/text/iter.html

5/ use list comprehension to create filtered list:

for shelf in [s for s in shelves2 if s['Length'] <= s['Width']]:
    ...


Hope it helps somehow.

jm