Responses inline.
On Apr 24, 2014, at 8:59 PM, Tom Hayward - esarfl@gmail.com wrote:
On Thu, Apr 24, 2014 at 8:09 PM, chirp.cordless@xoxy.net wrote:
Patch attached. -dan
I haven't tested it yet, but I see a few issues with style.
- Don't mix tabs and spaces. We prefer spaces.
Sorry, I will try to pay more attention to that. As an old-school C hacker, tabs are instinctive, but ever since some of my earlier Python attempts didn't work because of indenting issues, I've really tried to use only spaces. Guess I failed here. Do you have any hints for making this visible?
- 0 is a number. You can (and should) use True and False in Python to
represent booleans. This is a style thing, but it becomes important when another developer does a test like "1 is True". This evaluates False because the "is" operator is testing identity; 1 and True are different things.
Okay. Understood.
Here's a Python trick you may not be aware of:
_file_has_rTone = 0
_file_has_cTone = 0
for header in headers:
- if header == "rToneFreq":
_file_has_rTone = 1
- elif header == "cToneFreq":
_file_has_cTone = 1
Is equivalent to: _file_has_rTone = "rToneFreq" in headers _file_has_cTone = "cToneFreq" in headers
Short and sweet. It will iterate the list twice instead of once, but I think that's acceptable for readability.
I didn't know the idiom, and certainly way more readable. I'd actually felt a little guilty about iterating the header line for every line in the file, but didn't see a way to factor that without creating globals, which I didn't think would be wanted. If doubling that again for readability is preferred, I have a better calibration of the tradeoffs expected.
You didn't ask for a patch respin, shall I, or is this OK and I should just take this as advice for future work?
Thanks and regards,
-dan