Fwd: [Tutor] redundant function ( or, I need advice about cleaning up my code )

[Max Noel]

(argh, clicked "reply" again. I *will* get used to this one day, I 
swear)


Begin forwarded message:

> From: Max Noel <maxnoel_fr at yahoo.fr>
> Date: October 31, 2004 00:49:56 BST
> To: Morgan Meader <morgan at insightmill.com>
> Subject: Re: [Tutor] redundant function ( or, I need advice about 
> cleaning up my code )
>
>
> On Oct 13, 2004, at 20:40, Morgan Meader wrote:
>
>>        if job.matrix.info['gROWtype'][i] == 'power_ground' or \
>>           job.matrix.info['gROWtype'][i] == 'mixed' or \
>>           job.matrix.info['gROWtype'][i] == 'signal':
>
> 	This test can be shortened somewhat by using the in operator.
>
> 	if job.matrix.info['gRowType][i] in ('power_ground', 'mixed', 
> 'signal'):
> 		# do stuff
>
>>    layers.sort()
>
> 	Mmh... I don't see an initialization for this variable... Is it a 
> global variable? If so, it's a Bad Thing.
>
>>            if layerTypes[i] != 'signal':
>>                if layerPols == 'positive':
>>                    step.affect(layerNames[i])
>
> 	You can do that with a single test.
>
> 	if layerTypes[i] != 'signal' and layerPols == 'positive':
> 		step.affect(layerNames[i])
>
> 	If the first part of the test evaluates to False, the second part 
> won't be evaluated, as there is no need to (we already know the 
> boolean expression is false).
>
>
>
>
> 	Also, there are a few things that I think could be improved in your 
> general coding style:
> - Use functions. Lots of them. This single function you submitted to 
> us is huge. It could probably be broken up into at least 5 parts.
> - You're almost only using C-style for loops. It works, but you should 
> try to use more foreach-style for loops. Instead of doing:
> 	for i in range(len(a)):
> 		# do something with a[i]
> You should use:
> 	for element in a:
> 		# do something with element
> It's faster, more elegant and usually more efficient. Have a look at 
> list comprehensions, too, they're very useful.
> - I'm not absolutely sure, but it seems that some of the functions 
> you're using are modifying their arguments instead of returning 
> values. That's not a good idea, especially since Python can return 
> arrays, dictionaries or tuples.
>
> HTH,
>
> -- Wild_Cat
> maxnoel_fr at yahoo dot fr -- ICQ #85274019
> "Look at you hacker... A pathetic creature of meat and bone, panting 
> and sweating as you run through my corridors... How can you challenge 
> a perfect, immortal machine?"
>
>
-- 
maxnoel_fr at yahoo dot fr -- ICQ #85274019
"Look at you hacker... A pathetic creature of meat and bone, panting 
and sweating as you run through my corridors... How can you challenge a 
perfect, immortal machine?"