szhu / 3030

%%30%30 Game: Don't touch the trees! (Thanks, Chrome dev team!)
http://szhu.github.io/3030/
1.51k stars 114 forks source link

removed make_game_iter #26

Closed avamsi closed 2 years ago

avamsi commented 8 years ago

make_game_iter is kind of useless. str.join method creates a list anyway.

szhu commented 8 years ago
  1. I intentionally do want to process the file line-by-line. In the current version of game.map, you'll find there are lines of text that are treated differently. A simple find-and-replace won't do, since it'll replace the spaces in the sentences.
  2. Your new code takes less lines, but my code is (arguably) more readable: each line does only one thing, and it's easy to trace back output: each yield statement effectively outputs one line.
  3. Not only do I want the code to be readable, but I also want to make it so that when most people have some idea about how to change the program, they have to make changes in as few places as possible. That's why game.map is separate from game.py, and that's why the images aren't hardcoded into the map.

    I see you changed ' '.join(table[char] for char in in_line) to essentially gmap.replace(' ', table[' ']).replace('.', table['.']). Imagine if someone wanted to add a feature that allows the map to have not only two kinds of things, but a lot more. Dynamically looking up characters in table would be a better option than hardcoding each choice in the code.

I don't believe my code or principles are perfect; these are just my thoughts, and I'm telling you them so you can judge for yourself. If you think you have stronger reasons against these, please let me know!

avamsi commented 8 years ago

Hi @szhu I used replace as there are just 2 kinds of things. Yes, join would be better when there are more kinds of things.

But join needs needs to make two passes over the data and passing make_game_iter to join is unnecessary.

szhu commented 8 years ago

You're absolutely right in that join requires two passes.

However, because writing the file is atomic (I don't want to deal with the possibility of an error occurring when writing the file), we need to keep around the string that represents the entire file. Therefore, we kinda need to pass through the entire file contents at least twice, and adding an extra pass will not increase computation complexity, and so there will not be a very noticeable performance hit.

If I wanted make it faster, the better way is to have print directly write to the file.