[Tutor] problem solving with lists: final (amateur) solution

marcus.luetolf at bluewin.ch marcus.luetolf at bluewin.ch
Mon Jul 18 05:59:51 EDT 2022


Hello Experts, hello dn,
after having studied your valuable critiques I revised may code as below.

A few remarks: 
The terms "pythonish" and "dummy_i" I got from a Rice University's online lecture on python style.

If there is concern about a reader to have to "switch gears" between reading a for loop and a list comprehension then the concern
should be even greater to read a nested list comprehension coding the 4 flights for day 1.
I've read that a list comprehension should not exceed one line of code, otherwise a for loop should be used in favor of readability.

I'am quite shure that my revised code does not meet all critique points especially there are still separate code parts (snippets?) for
day_1 and day_2 to day_5 but not separate functions for at present I'am unable to "fold them in".

I also updated my code on github accordingly and adjusted docstrings and comments: 
https://github.com/luemar/player_startlist/blob/main/start_list.py

def start_list(all_players, num_in_flight, num_days_played):
    print('...............day_1................')
    history = {'a':[], 'b':[],'c':[],'d':[],'e':[],'f':[],'g':[],'h':[],\
              'i':[],'j':[],'k':[],'l':[],'m':[],'n':[],'o':[],'p':[]}
    
    for lead_player_index in range(0, len(all_players), num_in_flight):
        players = all_players[lead_player_index: lead_player_index + num_in_flight]
        [history[pl_day_1].extend(players) for pl_day_1 in players]
        print(all_players[lead_player_index] + '_flight_day_1:',players)

    for i in range(num_days_played - 1):
        flights = {}
        c_all_players = all_players[:]
        print('...............day_' + str(i)+'................')
        flights['a_flight_day_'+str(i+2)]= []
        flights['b_flight_day_'+str(i+2)]= []
        flights['c_flight_day_'+str(i+2)]= []
        flights['d_flight_day_'+str(i+2)]= []            
        lead = list('abcd')                   
        flight_list = [flights['a_flight_day_'+str(i+2)], flights['b_flight_day_'+str(i+2)],\
                       flights['c_flight_day_'+str(i+2)], flights['d_flight_day_'+str(i+2)]]

        for j in range(len(flight_list)):            
            def flight(cond, day):
                for player in all_players:
                    if player not in cond:
                        day.extend(player)
                        cond.extend(history[player])
                        history[lead[j]].extend(player)
                day.extend(lead[j])
                day.sort()
                [history[pl_day_2_5].extend(day) for pl_day_2_5 in day[1:]]
                return lead[j]+'_flight_day_'+str(i+2)+ ': ' + str(flight_list[j])
               
            conditions = [history[lead[j]], history[lead[j]] + flights['a_flight_day_'+str(i+2)],\
                          history[lead[j]] + flights['a_flight_day_'+str(i+2)] + \
                          flights['b_flight_day_'+str(i+2)], \
                          history[lead[j]] + flights['a_flight_day_'+str(i+2)] + \
                          flights['b_flight_day_'+str(i+2)]+ flights['c_flight_day_'+str(i+2)]] 
            print(flight(list(set(conditions[j])), flight_list[j]))
num_in_flight = 4
if num_in_flight != 4:
    raise ValueError('out of seize of flight limit')
num_days_played = 5
if num_days_played >5 or num_days_played <2:
    raise ValueError('out of playing days limit')
all_players = list('abcdefghijklmnop')
start_list(all_players,num_in_flight , num_days_played)

Many thanks and regards, Marcus.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------

-----Ursprüngliche Nachricht-----
Von: dn <PythonList at DancesWithMice.info> 
Gesendet: Sonntag, 26. Juni 2022 02:35
An: marcus.luetolf at bluewin.ch; tutor at python.org
Betreff: Re: AW: [Tutor] problem solving with lists: final (amateur) solution

On 26/06/2022 06.45, marcus.luetolf at bluewin.ch wrote:
> Hello Experts, hello dn,
> it's a while since I - in terms of Mark Lawrence - bothered you with 
> my problem.
> Thanks to your comments, especially to dn's structured guidance I've 
> come up with the code below, based on repeatability.
> I am shure there is room for improvement concerning pythonish style 
> but for the moment the code serves my purposes.
> A commented version can be found on
> https://github.com/luemar/player_startlist.
> 
> def startlist(all_players, num_in_flight):
>     c_all_players = all_players[:]
>     history = {'a':[], 'b':[],'c':[],'d':[],'e':[],'f':[],'g':[],'h':[],\
>                'i':[],'j':[],'k':[],'l':[],'m':[],'n':[],'o':[],'p':[]}
>     print('...............day_1................')
>     def day_1_flights():
>         key_hist = list(history.keys())
>         c_key_hist = key_hist[:]
>         for dummy_i in c_key_hist:
>             print('flights_day_1: ', c_key_hist[:num_in_flight])
>             for key in c_key_hist[:num_in_flight]:
>                 [history[key].append(player)for player in 
> c_all_players[0:num_in_flight]]
>             del c_key_hist[:num_in_flight]
>             del c_all_players[0:num_in_flight]
>     day_1_flights()
>     
>     def day_2_to_5_flights():
>         flights = {}
>         for i in range(2,6):
>             print('...............day_' + str(i)+'................')
>             flights['a_flight_day_'+str(i)]= []
>             flights['b_flight_day_'+str(i)]= []
>             flights['c_flight_day_'+str(i)]= []
>             flights['d_flight_day_'+str(i)]= []            
>             lead = list('abcd')                   
>             flight_list = [flights['a_flight_day_'+str(i)], 
> flights['b_flight_day_'+str(i)],\
>                            flights['c_flight_day_'+str(i)], 
> flights['d_flight_day_'+str(i)]]
> 
>             for j in range(len(flight_list)):            
>                 def flight(cond, day):
>                     for player in all_players:
>                         if player not in cond:
>                             day.extend(player)
>                             cond.extend(history[player])
>                             history[lead[j]].extend(player)
>                     day.extend(lead[j])
>                     day.sort()
>                     [history[pl].extend(day) for pl in day[1:]]
>                     return lead[j]+'_flight_day_'+str(i)+ ': ' +
> str(flight_list[j])
>                    
>                 conditions = [history[lead[j]], history[lead[j]] + 
> flights['a_flight_day_'+str(i)],\
>                               history[lead[j]] + 
> flights['a_flight_day_'+str(i)] + \
>                               flights['b_flight_day_'+str(i)], \
>                               history[lead[j]] + 
> flights['a_flight_day_'+str(i)] + \
>                               flights['b_flight_day_'+str(i)]+ 
> flights['c_flight_day_'+str(i)]]
>                 print(flight(list(set(conditions[j])), flight_list[j]))
>     day_2_to_5_flights()
> startlist(list('abcdefghijklmnop'), 4)
> 
>  Many thanks, Marcus.
...

> The word "hardcoded" immediately stopped me in my tracks!
> 
> The whole point of using the computer is to find 'repetition' and have 
> the machine/software save us from such boredom (or nit-picking detail 
> in which we might make an error/become bored).
...

> The other 'side' of both of these code-constructs is the data-construct.
> Code-loops require data-collections! The hard-coded "a" and "day_1" 
> made me shudder.
> (not a pretty sight - the code, nor me shuddering!)
...
> Sadly, the 'hard-coded' parts may 'help' sort-out week-one, but (IMHO) 
> have made things impossibly-difficult to proceed into week-two (etc).
...


It works. Well done!
What could be better than that?



[Brutal] critique:

- «pythonish» in German becomes «pythonic» in English (but I'm sure we all understood)

- position the two inner-functions outside and before startlist()

- whereas the «4», ie number of players per flight (num_in_flight), is defined as a parameter in the call to startlist(), the five «times or days» is a 'magic constant' (worse, it appears in day_2_to_5_flights() as part of «range(2,6)» which 'disguises' it due to Python's way of working)

- the comments also include reference to those parameters as if they are constants (which they are - if you only plan to use the algorithm for this 16-4-5 configuration of the SGP). Thus, if the function were called with different parameters, the comments would be not only wrong but have the potential to mislead the reader

- in the same vein (as the two points above), the all_players (variable) argument is followed by the generation of history as a list of constants («constant» cf «variable»)

- on top of which: day_1_flights() generates key_hist from history even though it already exists as all_players

- the Python facility for a 'dummy value' (that will never be used, or perhaps only 'plugged-in' to 'make things happen') is _ (the under-score/under-line character), ie

    for _ in c_key_hist:

- an alternative to using a meaningless 'placeholder' with no computational-purpose, such as _ or dummy_i, is to choose an identifier which aids readability, eg

    for each_flight in c_key_hist

- well done for noting that a list-comprehension could be used to generate history/ies. Two thoughts:

  1 could the two for-loops be combined into a single nested list-comprehension?

  2 does the reader's mind have to 'change gears' between reading the outer for-loop as a 'traditional-loop' structure, and then the inner-loop as a list-comprehension? ie would it be better to use the same type of code-construct for both?

- both the code- and data-structures of day_1_flights() seem rather tortured (and tortuous), and some are unused and therefore unnecessary.
Might it be possible to simplify, if the control-code commences with:

for first_player_index in range( 0, len( all_players ), num_in_flight ):
    print( first_player_index,
           all_players[ first_player_index:
                        first_player_index+num_in_flight
                      ]
         )

NB the print() is to make the methodology 'visible'.

- the docstring for day_1_flights() is only partly-correct. Isn't the function also creating and building the history set?

- that being the case, should the initial set-definition be moved inside the function?

- functions should not depend upon global values. How does the history 'pass' from one function to another - which is allied to the question:
how do the functions know about values such as _all_players and num_in_flight? To make the functions self-contained and «independent», these values should be passed-in as parameters/arguments and/or return-ed

- much of the above also applies to day_2_to_5_flights()

- chief concern with day_2_to_5_flights() is: what happens to d_flight_day_N if there are fewer/more than four players per flight, or what if there are fewer/more than 5 flights?

- the observation that the same players would always be the 'lead' of a flight, is solid. Thus, could the lead-list be generated from a provided-parameter, rather than stated as a constant? Could that construct (also) have been used in the earlier function?

- we know (by definition) that flight() is an unnecessary set of conditions to apply during day_1, but could it be used nonetheless? If so, could day_1_flights() be 'folded into' day_2_to_5_flights() instead of having separate functions?
(yes, I recall talking about the essential differences in an earlier post - and perhaps I'm biased because this was how I structured the
draft-solution)


[More than] enough for now?
--
Regards,
=dn



More information about the Tutor mailing list