[Tutor] Feedback on coding style

Dave Angel davea at ieee.org
Wed Dec 8 23:01:05 CET 2010


On 01/-10/-28163 02:59 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?
>
> PS: The script very simple accepts 2 arguments from the commandline.
> 	First arg being the number to which should be counted,
> 	second arg being the interval.
>
> Thank you,
> Adrian

I agree with Hugo, probably on all his points.  But to help you fix it, 
I think I can help you a bit as well.

First is I think you're misunderstanding the purpose of a function.  A 
function should be a self-contained piece of code that gets called, and 
that returns when done.  You're using them mostly as a way to implement 
what BASIC used to have as a GOTO.  For example, when a loop is needed, 
you do it by calling another function which calls the first one.  That 
is called recursion if it's done on purpose, and a bug if it happens by 
accident.

Second is that you're misusing global variables.  A function needs to be 
called with those values it needs to do its work, and it needs to return 
the results of that work.  Very seldom should those things be globals.

Start with the first function.  You declare it with maxn and incr as 
parameters, and you call it correctly.  But it also uses the globals, 
rather than using the ones passed in.

Then the function ask_change().  You should be passing it the two 
arguments, and getting back modified arguments as return values.  And 
the function shouldn't call the_loop() itself, it should just get the 
new values.

jibjab_result() is trying to be a loop, by effectively calling itself, 
through choice_result().  If you need a loop, just write one.  And that 
loop will probably be in ask_change(), without needing any other 
functions inside.

It's good to decompose a problem into functions, but you're not using 
functions in the way they're designed.  For small problems, this can 
work, but as the problems get more complex, you'll be forced to change 
your habits.

DaveA





More information about the Tutor mailing list