Source Code Makeover: Square Shooter, Part 1 | Hacker News Reader

Source Code Makeover: Square Shooter, Part 1

inventwithpython.com

52 pointsyen22312 comments

> 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... 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).

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.

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.

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.

I did find it strange that the post didn't even address that the allignment advice was contrary to PEP 8. That being said, I definintely find

  my_data   = 3
  your_data = 2

much easier to skim than

  my_data = 3
  your_data = 2

Given that PEP 8 even says "But most importantly: know when to be inconsistent -- sometimes the style guide just doesn't apply. When in doubt, use your best judgment. Look at other examples and decide what looks best." I have no problem occasionally using more than one space for allignment.

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.

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?

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.

Yes, most of them are normal coding practices. But ways to write better code generally are or at least should be normal coding practices. But many people do not know what the normal practices should be.

The lowest quality of code I have ever seen has been tutorial code. In my experience(of reading various tutorials) it is uniformly badly written. There is never any error detection, basic things like setting up constants are relatively rare, Logging and debugability is never considered.

    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.

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

Reply on news.ycombinator.com