[chirp_devel] [csv] Friendlier defaults for missing cToneFreq/rToneFreq columns in csv import - Fixes #1577

[chirp.cordless at xoxy.net]

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 at 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 the
>> +        csv file, use it for both rtone & ctone. Makes TSQL use friendlier."""
>> +
>> +        if not hasattr(self, "_file_has_rTone"):
>> +            self._file_has_rTone = "rToneFreq" in headers
>> +        if 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