Much of the Gramps code uses the style foo = get_person_from_handle(bar).
When we read this code, and the code around it, it is completely clear that foo is a Person object. We don’t need the name of the foo identifier to be sure about this. The called method doesn’t need the complexity or performance penalty of singledispatch or if elif to find out what object type we are talking about. Static type checking is simple, because foo is certainly a Person object.
[In fact, static type checking has clarified that this method should always return a Person object, or raise an exception, and this has improved the logic of Gramps. Static type checking has also raised the question as to whether the method should be get_person_from_person_handle to match the static type of the handle. However, I am not so concerned about this, because all handles are actually of the same ‘real’ type, which is not so with Gramps objects and if the handle is wrong it will raise an exception].
However, now we have methods like foo = data_to_object(bar, baz). It’s not intuitively clear whether bar or baz are supposed to be the object type. In fact baz is optional, and is omitted in the general run of Gramps code, so we have no idea what type foo is supposed to be as we read the code. Of course the code author knows the type of the object so this should be documented in the name of the called method. Even worse in this case is that bar can actually be either a DataDict or a tuple, so reading the code we don’t know what it is supposed to be, and the called method has to check what bar actually is.
[data is even more confused by the fact that throughout the code different names are used for different and the same things, like data, dict, RawData, string, raw_person etc. for the different formats that are not objects, whether tuple or DataDict or JSON or pickled data. I appreciate that there is a lot of code to change, but perhaps this could be started before the general run of code starts to use the DataDict etc formats more extensively. I suggested new names ‘struct data’ (but now we need to differentiate between the two types of data) and ‘raw data’ so they were different from any existing names so that change to terminology could be introduced gradually.]
Another point is that if we want to insert code to check whether the passed parameter is correct, for example to check that a Person object has all, and only, the expected properties, either for a temporary diagnostic check, or even more permanently, we cannot do so if we do not know what type is expected to be passed in.
I also noticed the recent comment Grampstype set #1830 where the set method was converted to singledispatch, and once again the called method has to laboriously find out what the the parameter actually is.
I am not sure what impact the lack of clarity in method naming has on static typing (not good I imagine), but it certainly makes the code less clear.
This is a followup to the thread in the mailing list
[Gramps-devel] Switch from pickled blobs to JSON data #1786 - Abstraction. I am hopeful that something similar to the suggested changes can be made.
Tim Lyons
