

Source Code Makeover: Square Shooter, Part 1 - yen223
http://inventwithpython.com/blog/2012/08/09/source-code-makeover-square-shooter-part-1/

======
zalew
> When there is nearly identical code, I like to make the spacing match up so
> it easier to compare them with each other.

When you make a tutorial how to write better code in Python, you should
follow, you know, Python conventions how to write code.
[http://www.python.org/dev/peps/pep-0008/#whitespace-in-
expre...](http://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-
and-statements) The same with those oneline conditionals, lack of spacing
between functions, etc. Following PEP8 (with --ignore=E5,W602) is useful (if
not for you, then for others who will commit to your code) and easy (configure
your ide).

~~~
eieio
I try very hard to follow PEP 8, but I have noticed some very nice python
code(such as the setup portion of Peter Norvig's _beautiful_ sudoku code
<http://norvig.com/sudoku.html>) that alligns equals signs using more than one
space. I try to use allignment when it makes the code easier to read, but if
I'm unsure I will defer to PEP 8 and not allign.

~~~
zalew
if you care enough make a tutorial, you should care enough promote best
practices, just sayin'.

I ignored pep8 for a long time, until I noticed more and more whitespace mess
in diffs. Following pep just makes work easier.

~~~
famousactress
Saying "best practices" makes my brain turn off. Call it 'dogma', it's more
accurate because it lacks the subjective boolean people like to slip behind
'practices'.

I get it. PEP-8 is rad, and pretty well accepted... but:

1\. It's a blog post. It's subjective opinion. 2\. Whitespace alignment is
rad, and some of the PEP-8 whitespace rules are pretty unfortunate.

I guess if it were me I'd at least have called out that the alignment violates
PEP-8 as a warning to folks.. and I wouldn't have led the tutorial with it, so
that I'd at least be dealing with trolls that make it through the entire post.

------
tomku
I can't help but feel that the powerup_type change is short-sighted. In the
new code, the enumeration of powerup types happens three times, in three
different places. The first is in Powerup's constructor as a tuple of strings,
the second is in Powerup.render as three paths of an if/else, and the third is
in GameWorld.apply_powerup as the keys to a hash of functions.

It feels wrong that GameWorld should keep track of a list of powerup types
independently from instances of the Powerup class, and that Powerup instances
have no knowledge of what they do. It smells suspiciously like using strings
as a substitute for object types. To me, that hints that there are stronger
refactorings to be found that could take advantage of polymorphism or a
strategy pattern, depending on your taste for inheritance hierarchies.

------
Eduardo3rd
I'm just a non-computer scientist who mainly codes in Python and Matlab, but a
lot of these suggestions (making a number a vairable if it is used more than
once, etc) seem like normal coding practices to me. Do very many coding
tutorials leave out this sort of thing?

~~~
yen223
The tips written there should (hopefully) be fairly obvious to seasoned
programmers, I agree.

However, I find that this is particularly helpful in that it based on an
actual working code base, instead of contrived examples. It demonstrates that
writing clean code isn't just a pointless academic exercise, there is a
purpose to it.

------
ralph

        def wrap_around(self):
            """Change the position of the bubble to toroidally
                "wrap around" if it goes off one edge of the
                map."""
            if self.pos.x < 0: self.pos.x += 1
            if self.pos.y < 0: self.pos.y += 1
            if self.pos.x > 1: self.pos.x -= 1
            if self.pos.y > 1: self.pos.y -= 1
    

Why isn't else used? In addition to efficiency it tells the reader that the
author expects at most one of x++ or x-- to happen.

------
jarcoal
Nice post, thanks. However, as long as we're in best practices mode, you may
want to write those commit messages in present tense.

