[Tutor] What's in a name?

[Danny Yoo]

On Thu, Jan 2, 2014 at 10:28 PM, Keith Winston <keithwins at gmail.com> wrote:
> Danny: I appreciate your point, but these are just for little code loops,
> nothing I need to hold on to, like the snippet above:

I hope you don't take offense.  But I actually do not understand
print_candl_info().  I thought I did!  But then I realized I was
deluding myself.  I don't not understand it after all, and that's
after staring at it for more than about thirty minutes.

Take that a "canary in the coalmine" kind of comment.  eval() can
encourages habits that hurt program readability in a deep way.


[Note: the rest of this message is a relentless refactoring of the
original code, step by step.]



If you don't mind, I'd like to iteratively strip away the eval() and
see if we can't make it a little easier to understand.  From a first
treatment, I think this might look something like the following:

########################################################
def print_candl_info(garray):
    game_array, chute_nums, ladder_nums = {}, chutes, ladders
    for i in chute_nums.keys(): chute_nums[i] = 0
    for i in ladder_nums.keys(): ladder_nums[i] = 0

    for corl in chute_nums:
        for game in garray:
            if corl in game[4]:
                chute_nums[corl] += 1
    print("total ", "chute_nums", "= ", sum(chute_nums.values()))

    for corl in ladder_nums:
        for game in garray:
            if corl in game[5]:
                ladder_nums[corl] += 1
    print("total ", "ladder_nums", "= ", sum(ladder_nums.values()))
########################################################

where we first unroll the original's outer loop out so that there are
no eval()s anywhere.  This hurts program length a little, but we can
deal with that.  Do you agree so far that the program above preserves
the meaning of what you had before?



If we can assume that the rewritten code "means" the same thing, then
we can eliminate the duplication by doing something like this:

########################################################
def print_candl_info(garray):
    game_array, chute_nums, ladder_nums = {}, chutes, ladders
    for i in chute_nums.keys(): chute_nums[i] = 0
    for i in ladder_nums.keys(): ladder_nums[i] = 0

    summarize_game("chutes", chute_nums, 4, garray)
    summarize_game("ladders", ladder_nums, 5, garray)

def summarize_game(game_name, game_nums, game_index, garray):
    for corl in game_nums:
        for game in garray:
            if corl in game[game_index]:
                game_nums[corl] += 1
    print("total ", game_name, "= ", sum(game_nums.values()))
########################################################

where we put the the duplicated part in a function, and then call that
function twice, once for chutes, and the other for ladders.


Hmm... it's a little easier to see from this point that the "zero all
values" part also appears to be duplication.


Maybe that can be absorbed into the front of summarize_game?  Let's
iterate again.  Here's what it looks like afterwards:

########################################################
def print_candl_info(garray):
    game_array, chute_nums, ladder_nums = {}, chutes, ladders
    summarize_game("chutes", chute_nums, 4, garray)
    summarize_game("ladders", ladder_nums, 5, garray)

def summarize_game(game_name, game_nums, game_index, garray):
    for i in game_nums.keys(): game_nums[i] = 0

    for corl in game_nums:
        for game in garray:
            if corl in game[game_index]:
                game_nums[corl] += 1
    print("total ", game_name, "= ", sum(game_nums.values()))
########################################################


Reading...

Hmmm.  I don't understand why chute_nums is assigned to chutes, nor
ladder_nums to ladders, and I don't see game_array being used here
yet.  We can absorb that:


########################################################
def print_candl_info(garray):
    summarize_game("chutes", chutes, 4, garray)
    summarize_game("ladders", ladders, 5, garray)

def summarize_game(game_name, game_nums, game_index, garray):
    for i in game_nums.keys(): game_nums[i] = 0

    for corl in game_nums:
        for game in garray:
            if corl in game[game_index]:
                game_nums[corl] += 1
    print("total ", game_name, "= ", sum(game_nums.values()))
########################################################


Hmmm...  I think that's about as far as I can go with zero domain knowledge.  :P

At this point, the code makes a little more sense to me, though the
data structure still feels like a black box.  That is, I don't know
the shape of the data for 'garray' or 'corl'.  If we knew more about
the structure of the data, maybe this could be further improved.



As a side note: the code above has more latitude than the original
because it can print out a friendlier game name.  That is, for
example, you can do this:

##################################################
    summarize_game("OOOO Chutes! OOOO", chutes, 4, garray)
    summarize_game("HHHH Ladders! HHHH", ladders, 5, garray)
##################################################