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

[dn]

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