From: Benny M. <ben...@gm...> - 2009-12-09 14:15:55
|
2009/12/9 Brian Matherly <br...@gr...>: > Doug, > >> > Explicit coupling occurs when a dependency occurs >> between two modules because one module uses another module. >> For example, a module requires that in order to call a >> function, you have to pass in an enumerated type that is >> also defined in that module, the dependency on that module >> is explicit. >> > >> > Example: eventref.set_role(EventRoleType.BRIDE) >> > >> > If however, instead of passing in an enumerated type, >> you pass in a magical string constant, the dependency still >> exists, but it is not explicit. >> > >> > Example: eventref.set_role("Bride") >> >> I agree that this code needs to be tightly coupled... > > I didn't say it is tightly coupled. In fact, there isn't enough enough information to know whether the coupling is tight or lose. All we know is that it is coupled. Coupling isn't binary (loose coupling = no coupling, tight coupling = some coupling). Rather, it is a continuum. All we can say about the code above is that one dependency exists. If that is the only dependency, the code is loosely coupled. If there are many more, the code may be tightly coupled. > >> but I disagree >> on your argument. In the end, the text between the >> parentheses is a >> "magical value". One is a int and one is a string. > > I definitely disagree. The int is not magical because it is defined as part of the interface in the class definition. There is nothing magical about it at all. I can look at the EventRoleType interface and instantly see what values are valid. > > Only the "Bride" string is magical because the only way to find out what the valid values are is to inspect the internal workings of EventRef, and all the classes that depend on it. > >> The important part >> is that the two pieces of code (the setter and the eventref >> object) >> both agree, thus the tight coupling. (Of course, I'm >> assuming that >> both setters would do checking to make sure that it is a >> valid value.) >> >> But I would advocate the second, as it has a lower mental >> load on the >> developer. The first requires an import (where is that >> value defined?) > > I argue that the mental load is lower when using the enumeration because the interface tells you where it is defined. > > If using an arbitrary string, the mental load is higher because I have to inspect the EventRef code to find out what valid values exist - this take more time and is more difficult to reference. > >> and you have to remember (or lookup) the integer's name. >> The string >> setter just requires the name, which is easier to look at >> for the >> human ("Bride" vs BRIDE). >> >> But an even more important reason why I would choose the >> second is >> that you can't bypass the readability. For example, the >> first I could >> do this: >> >> Example: eventref.set_role(5) > >> That is valid and completely unreadable. You can't do that >> with the >> second method. > > Sure you can: > > val = "bride" > > ...[lot and lots of code]... > > eventref.set_role(val) > > the readability is equally bad. > >> I don't think that the above examples have anything to do >> with what >> I'm advocating (although I would advocate also for the >> lower cognitive >> load on the programmer, too). These two different >> techniques (lower >> cognitive load, loose coupling) may look similar, but don't >> really >> have anything to do with each other (except they both use >> strings, >> perhaps). > > They have everything to do with each other - when your idea of lower cognitive load is increasing the coupling between modules (implicitly) > >> There are some things that the user should be able to add >> and remove >> without changing anything in gramps. For example, if I >> wanted to add a >> new View called "Current" and have it show what I am >> currently working >> on, I shouldn't have to create a new view category. > >> With a plugin system, we should have some things be >> coupled >> (EventRoleTypes) and those that shouldn't. I can't think of >> a reason >> why we would want to limit the kinds of gramplets. I would >> leave many >> more things uncoupled as well. >> >> Anarchy? Chaos? No, it doesn't have to be "anything >> goes"... there >> would still be agreed upon conventions. But if one wanted >> to evolve a >> new type of report (for example) it should not require >> editing 35 >> files. It should require editing zero files. > > I agree absolutely. I only disagree with your strategy. You can accomplish the above by making clean interfaces that meet your needs without creating so many implicit dependencies. You just have to put some thought into the interface when designing it. > >> Anyway, thanks for the extended example (below) and >> elucidation on >> your philosophy. It makes it easier to see where you are >> coming from. > > No problem. Have a great day. To continue with the example, the code was changed to category = ("Charts", _("Charts")), from category = CHART Where the API of pluginreg now reads: Attributes for REPORT and TOOL and QUICKREPORT and VIEW plugins .. attribute:: category Or the report category the plugin belongs to, default=CATEGORY_TEXT or the tool category a plugin belongs to, default=TOOL_UTILS or the quickreport category a plugin belongs to, default=CATEGORY_QR_PERSON or the view category a plugin belongs to, default=("Miscellaneous", _("Miscellaneous")) Ok, category at the moment is messy because plugindata is one class except of a class per plugintype, but that is not difficult to change, the code is organised in logic parts per plugin type already (I would prefer it remains one file though, I hate having to chase many small files for obvious code inheritance). To satisfy Brian, the API should indicate all valid values, so eg for report: Report category can be one of CATEGORY_TEXT, CATEGORY_DRAW, CATEGORY_CODE, CATEGORY_WEB, CATEGORY_BOOK, CATEGORY_GRAPHVIZ. Tool category can be .... The way views are done now is with strings. I personally don't like that also, so I agree with Brian. I like to be able to read in the API of the registration code what is possible, and how free extensions can be added. I would rather see preknown views as enumeration types (VIEW_PERSON, ....), and then to allow not predefined view categories a specific VIEW_CUSTOM. For VIEW_CUSTOM, one then must provide the category name and it's translation. This still has the checks to avoid mistyping (allowed categories are checked), while explicitly allowing for plugin authors to add to the predefined categories. Having the predefined view categories in the config as register('interface.view-categories', ["Gramplets", "People", "Relationships", "Families", "Charts", "Events", "Places", "Geography", "Sources", "Repositories", "Media", "Notes"]) means the data is now in pluginreg.py, in all view gpr.py files, and in config.py as strings. I assume the addition to config means we now also have a way to know the order of categories, but this is a not documented side effect. Benny > > ~Brian > > > ------------------------------------------------------------------------------ > Return on Information: > Google Enterprise Search pays you back > Get the facts. > http://p.sf.net/sfu/google-dev2dev > _______________________________________________ > Gramps-devel mailing list > Gra...@li... > https://lists.sourceforge.net/lists/listinfo/gramps-devel > |