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

[Dan Smith]

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

It's not wrong, it's just generally not preferred.

> I think you're saying that on-the-fly addition of data items
> to an object is not preferred style?

Right, so the reasoning for this is that someone comes along later and
sees that "oh, there is a self._file_has_header_foo property" and goes
to use it in their separate method. However, if they don't realize that
it was only added in the middle of your method, which seems like it only
cleans things and isn't particularly structural, then they'll get a
crash if they run before you run.

Initializing it to *something* in __init__ just helps avoid that
situation. Note that pre-initializing things in a C-like way is very
unpythonic, but in this case it's just a "safety for others" sort of
thing. It's subtle, optional, and certainly open to opinion. However,
when you have a bunch of different folks working on the same set of
code, sometimes it helps to be a little more explicit about things like
that.

What you're adding is somewhat of a predicate, which always has some
value, even if "false until determined otherwise." That makes it even
more reasonable (IMHO) to have it be defined in some way, even if the
default state is "not sure yet, assume false" (i.e. the None case).

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

Yep.

> 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

Just set it once we know what it should be, which would be once we read
the headers.

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

Nope, the subclass should call the superclass' init, which will set
them. If they're never used, that's fine, and there is no need for the
subclass to pre- or post-define them in that case.

Again, python is super dynamic, as you saw from your use of modifying
self there in the middle of your method. I'm not trying to scare you
away from using the dynamic-ness, I just think that in this particular
case, it's worth being more explicit.

Thanks for humoring me :P

--Dan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
Url : http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20140428/70e8592d/attachment-0001.bin