I don't mind changing it if I understand what to change it to. The current code was Tom's suggestion after I made the comment
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.
I've said before I'm a bit of a newbie to both OOP and Python. The concepts of both on-the-fly addition of data items to an object, and conditionals on their existence (vice value) are new to me, and I just took it as the way it should be done.
I think you're saying that on-the-fly addition of data items to an object is not preferred style?
I do see that it could be initialized in the constructor (if that's the right term in Python) and so still not be a global, which is what I think you're saying.
By "when we start the load" do you mean in this clause in load(): if lineno == 1: header = line # add my test for rToneFreq/cToneFreq headers here continue
Would initializing my booleans to None in __init__ possibly end up raising an exception in some subclass with its own __init__() but not its own _clean_tmode()? Is that even asking a reasonable question? If not, ignore the next question.
Since my code doesn't do anything unless one is True and the other False, if I set them both in load(), would initializing them False in __init__() be more robust?
-dan
On Apr 27, 2014, at 6:23 PM, Dan Smith - dsmith@danplanet.com wrote:
I don't mean to nitpick, and I know I've ignored this for too long anyway. However, I don't really like this:
- def _clean_tmode(self, headers, line, mem):
""" If there is exactly one of [rToneFreq, cToneFreq] columns in thecsv file, use it for both rtone & ctone. Makes TSQL use friendlier."""if not hasattr(self, "_file_has_rTone"):self._file_has_rTone = "rToneFreq" in headersif not hasattr(self, "_file_has_cTone"):self._file_has_cTone = "cToneFreq" in headers...because it modifies self in the middle of a method. Would you mind if we set those in __init__, perhaps just to None, and then set them True/False when we start the load?
If you don't want to spin it again, I'll just apply as-is and chalk it up to OCD, but I really think it'd be nicer.
--Dan