[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