[Tutor] Feedback on coding style

[Hugo Arts]

On Wed, Dec 8, 2010 at 8:11 PM,  <howitzer at archlinux.us> wrote:
> Hi.
>
> For the past week, I've been following an online Python guide named:
> 'Learn Python the Hard Way'. I'm very happy with it as it gives a lot of
> freedom to explore.
>
> However, due to this I have no idea if I'm thinking the right way. That's
> why I've attached a script of mine I've been working on all day.
>
> It works a 100%, but I'm afraid I've made very bad choices concerning
> design and coding style. (If it could've been much simpler, if there are
> glaring mistakes, poor methods, ..)
>
> Could anyone be so friendly as to offer a bit of feedback, to a newbie?
>

I like that you've divided the program up into functions, this is A
Good Thing(TM). It made it much easier for me to understand what is
going on, so keep doing that.

little nitpick (maybe not *so* little), your functions are mutually
recursive. This is a difficult word that basically means that they are
all calling each other in a cycle that is potentially endless. In this
case, the cycle goes like this: check_OP_size -> ask_change ->
choice_result -> change_vars -> check_OP_size. You see that if I keep
inputting the right things, I can go through this cycle endlessly. In
this case, it's not a big problem, because no one is likely to do that
for long enough to cause problems, but if another part of your program
is doing the inputting, it can go wrong quickly. Why? Because
functions can't call each other indefinitely. Try running this
example:

def f(): f()
f()

This is a function that keeps calling itself, the simplest possible
cycle. It gives you an error "maximum recursion depth exceeded." There
are reasons for this, and you should learn about recursion and the
call stack at some point, but that is another long essay. For now it
is enough to know that recursion is something to be careful with, as
there is a limit to how many functions can call each other.

Second, you're using globals too much. Functions should rely as much
as possible on their arguments, and nothing else. I can count the
number of times I needed the global statement on one hand, and I've
programmed a lot of python. Globals make your program difficult to
follow, because you can't clearly see where the data is coming from
and going to. Rather than having your functions communicate through
global variables, you should have them communicate through function
arguments and return values. See if you can write a version without
the global statements.

There are a few other little things. The two if statements in
check_OP_size can be rewritten into an if/else statement, since the
two checks are complementary (one of them will always be true). The
while loop in the_loop is a little overcomplicated (you had the right
idea with the commented out range call).

Lastly, the program could've been written a lot more succinctly, but
this will come with time and practice. Writing code is hard, and
getting a feel for how to divide up your functions and how to express
yourself in python most naturally will take time and practice. I'd say
you're on the right track, and if you get rid of all the globals
you'll be well on your way to becoming an excelling python programmer.
If you need any more help, let us know.

Hugo