Question about class inheritance

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Question about class inheritance

Patrick Gerlier

To try and understand the logic in the support "functions" of Gramps, I am writing synthetic notes about the classes and their methods.

As a side remark, I don't find the raw list in the developer pages of the Gramps site very useful because they are a mere extraction of the code without added-value comments about typical usages and they don't show examples. Yes, I know, this a task lacking volunteers.

Starting with the GUI, I made a diagram for the Option objects. See attached file.

I am questioning the relevancy of making DestinationOption, FamilyOption, MediaOption, NoteOption and PersonOption inherit from StringOption.

StringOption is strictly equivalent to Option. Apart from the fact that this inheritances "types" the semantics of the __value field of the classes, I don't see the relevance.

  • Even if all __value's are of type string, they are used very differently. The string must be split according to class-specific rules to be used.
  • There is nothing specific to StringOption in all the uses of the subclasses (because StringOption adds no method to Option).
  • All other uses of StringOption are in the various report initialisation dialogs and are, there, really entry fields for one-line data.

Wouldn't it be simpler to have the aforementioned classes directly inherit from Option?

Would there be a slight performance improvement with this?

If my question appears naive, please forgive me: I don't practice Python and I'm only starting to discover Gramps internals.

Regards,
Patrick



_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel

Gramps_classes.pdf (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Question about class inheritance

prculley
You are correct that Option and StringOption are functionally the same.  And there might be a slight performance improvement, although where this is used it is unlikely to be noticeable.

I would guess that the original authors thought that adding the "String" to the class name would help make the code more clear in what the item was used for.

As for the others that inherit from StringOption, they are dealing with strings, as opposed to numbers etc.

Keep in mind the history; Gramps was started as a personal project for Don's own use.  But he understands how to use Python to be at least somewhat self documenting, at least for those who understand Python and have gotten used to some of the project ways of doing things.  It took me a couple of years of work, patching bugs etc. to get to where I am sort of familiar with most of the goodies.

And yes, our documentation is not up to professional standards; volunteers don't usually want to spend time on that sort of thing.  That said, we appreciate any articles anyone wants to write; I have made a couple, like https://gramps-project.org/wiki/index.php/Signals_and_Callbacks, for example.  And there are others like https://gramps-project.org/wiki/index.php/Gramps_Data_Model which try to cover the primary data classes.

If you are the sort to try to write notes as you come to understand something, and want to make articles of these notes, we applaud your work!

Paul C.

On Thu, Apr 9, 2020 at 8:32 AM Patrick Gerlier <[hidden email]> wrote:

To try and understand the logic in the support "functions" of Gramps, I am writing synthetic notes about the classes and their methods.

As a side remark, I don't find the raw list in the developer pages of the Gramps site very useful because they are a mere extraction of the code without added-value comments about typical usages and they don't show examples. Yes, I know, this a task lacking volunteers.

Starting with the GUI, I made a diagram for the Option objects. See attached file.

I am questioning the relevancy of making DestinationOption, FamilyOption, MediaOption, NoteOption and PersonOption inherit from StringOption.

StringOption is strictly equivalent to Option. Apart from the fact that this inheritances "types" the semantics of the __value field of the classes, I don't see the relevance.

  • Even if all __value's are of type string, they are used very differently. The string must be split according to class-specific rules to be used.
  • There is nothing specific to StringOption in all the uses of the subclasses (because StringOption adds no method to Option).
  • All other uses of StringOption are in the various report initialisation dialogs and are, there, really entry fields for one-line data.

Wouldn't it be simpler to have the aforementioned classes directly inherit from Option?

Would there be a slight performance improvement with this?

If my question appears naive, please forgive me: I don't practice Python and I'm only starting to discover Gramps internals.

Regards,
Patrick

_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel


_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Question about class inheritance

GRAMPS - Dev mailing list
>> Starting with the GUI, I made a diagram for the Option objects. See attached file.

That is a nice diagram. Very useful.

> I would guess that the original authors thought that adding the
> "String" to the class name would help make the code more clear
> in what the item was used for.

I think that was probably me. Thanks for assuming positive intentions on my part.

>> I am questioning the relevancy of making DestinationOption,
>> FamilyOption, MediaOption, NoteOption and PersonOption inherit
>> from StringOption.
>> StringOption is strictly equivalent to Option. Apart from the fact
>> that this inheritances "types" the semantics of the __value field
>> of the classes, I don't see the relevance.

While I can't recover my logic from 12 years ago, I'm sure it had more to do with clarifying the identify of the class than a real functional necessity.

I do not feel strongly about the inheritance. But I would suggest to leave it unchanged unless:
1) The change improves the clarity of the code
OR
2) There is a measured performance improvement (not just based on intuition or guess).

~Brian


_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Question about class inheritance

Patrick Gerlier
Hello Brian

Le 10/04/2020 à 21:45, Brian Matherly a écrit :

>>   I would guess that the original authors thought that adding the
>> "String" to the class name would help make the code more clear
>> in what the item was used for.
> I think that was probably me. Thanks for assuming positive intentions on my part.
>
>>> I am questioning the relevancy of making DestinationOption,
>>> FamilyOption, MediaOption, NoteOption and PersonOption inherit
>>> from StringOption.
>>> StringOption is strictly equivalent to Option. Apart from the fact
>>> that this inheritances "types" the semantics of the __value field
>>> of the classes, I don't see the relevance.
> While I can't recover my logic from 12 years ago, I'm sure it had more to do with clarifying the identify of the class than a real functional necessity.

This is where I (softly) disagree: the value of FamilyOption et al. is
effectively stored as a string, but reading the code you realise that
this string has a "restricted" semantics. It is not arbitrary, it is a
Gramps ID. Therefore I would rather have it directly inherit from Option
or from a GIDOption if you want to emphasize the semantics.

Personally, I"m not presently sufficiently fluent in Python and versed
in Gramps internal to make the change. I keep it as a remark.

In addition toi the class graph, I am writing very terse notes about the
classes. See the attached file. Comments are welcome. Note that I chose
to use a spreadsheet for my notes. This can easily be converted to an
HTML table to add to developer wiki (provided you find it useful).

Of course, this is only initial work-in-progress. There are many things
to improve (notably add Gramps code version number).

> I do not feel strongly about the inheritance. But I would suggest to leave it unchanged unless:
> 1) The change improves the clarity of the code
> OR
> 2) There is a measured performance improvement (not just based on intuition or guess).
This is wise.
>
> ~Brian
Patrick



_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Question about class inheritance

Patrick Gerlier
In reply to this post by GRAMPS - Dev mailing list
Sorry, forgot to attach the file

Patrick



_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel

Gramps_classes.ods (21K) Download Attachment