Jabref: Desired Architecture

[lenhard]

Following the discussion from today's devcall, here comes an outline of our desired high-level architecture for JabRef and some data on the status quo. This is a refinement from our high-level documentation.

@JabRef/developers Please have a look, mention anything that you find worth remarking or discussing, and eventually I will update the high level documentation to reflect our desired architecture. Here is a screenshot from my architecture modeling tool for JabRef 3.4 (apologies for overlapping arrows and numbers, there's no possibility to avoid this tool-wise):

Arrows mark dependencies and the number of an arrow reflects the number of dependencies. Dashed arrows mark forbidden dependencies, whereas complete arrows mark allowed dependencies. The model depicts the desired architecture, _not_ the actual architecture. I manually sorted all classes of jabref into these high-level packages, for instance the MetaData class should be in jabref.model and that would currently lead to forbidden dependencies from jabref.model to jabref.gui. Based on my sorting and the actual current source code, the architecture tool computes the number of dependencies between the packages.

The content of jabref.model, jabref.logic, and jabref.gui is described in our high-level documentation. On top of that jabref.globals should contain bootstrapping classes and global information (JabRefMain, Defaults, Globals, etc.). jabref.preferences should bundle the classes JabRefPreferences and JabRefPreferencesFilter. jabref.cli should bundle the CLI classes (already a top level package). According to the model, the only allowed dependencies in jabref should be:

jabref.logic -> jabref.model

jabref.globals -> everywhere

jabref.gui -> everywhere

jabref.cli -> jabref.preferences
jabref.cli -> jabref.globals
jabref.cli -> jabref.logic
jabref.cli -> jabref.model

This is an open discussion, any comments are welcome!

[oscargus]

Some comments/questions, primarily based on the experience from #1593:

• Where will the Stringconstants from Globals end up? These are:

    public static final String FILE_FIELD = "file";
    public static final String FOLDER_FIELD = "folder";
    public static final String DIR_SUFFIX = "Directory";
    public static final String SIGNATURE = "This file was created with JabRef";
    public static final String ENCODING_PREFIX = "Encoding: ";
    public static final String COL_DEFINITION_FIELD_SEPARATOR = "/";
    public static String NEWLINE = System.lineSeparator();
    public static final String SPECIAL_COMMAND_CHARS = "\"`^~'=.|";

It seems like FILE_FIELD is so established now that it is not needed to be in Globals and we can use "file" directly. FOLDER_FIELD is one used in one place in the code and I'm not even sure that the code does something useful. DIR_SUFFIX can probably be hard coded as well. SIGNATURE and ENCODING_PREFIX should be able to move to somewhere in logic.export. COL_DEFINITION_FIELD_SEPARATOR should, out of necessity, move to FieldComparator (as the other uses are in gui). NEWLINE is far from obvious to me how to handle. SPECIAL_COMMAND_CHARS could probably be moved to, say, StringUtil.

• Assuming that logic should not know about preferences, I assume that means that it is not even OK to pass the complete Globals.prefs to any logic method? (Hence, #1593 isn't really the solution.) However, I see one major issue here and it is layout which may need many different values from the preferences. I do not see it feasible to pass each individual possibly used preference value needed for any of the methods, as these are needed in a lot of places and there will be many functions to add arguments to, although in most cases the arguments will not even be of any value. I had a similar discussion regarding injection of JournalAbbreviator, but this may be even worse...

In general I think the general idea is really good though, but as I said, I do not really see how logic can not know about preferences. Also, now, there are specific preference classes, such as SaveSessionPreferences or OpenOfficePreferences, which reside in logic and I just cannot see where they should be. If they are in preferences they can not be used in logic, if they are in logic they cannot access preferences.

[oscargus]

Where should event end up?

[lenhard]

Thanks for your feedback!

The event package is relatively simple. Since it is tightly coupled to model classes, which throw the events, it should be located in the model package (model.event probably). This should be possible out of the box right now.

Regarding the constants in Globals: I think we should keep constants and not use hard coded values, even if FILE_FIELD is sufficiently established. If possible, we can try to move some constants closer to their actual usage, but that really depends on each constant individually. We can check the usages and if, for instance, a constant is only used in logic.export, then it can be moved into logic.export. If it is used in multiple places, it should just stay in Globals for now. If FOLDER_FIELD doesn't really do anything useful, we could maybe just delete it.

Regarding preferences: The (very very long-time) plan is to turn model and logic into an API and, therefore, it should be free from preferences. Any configurations needed in these packages should be passed as parameters. However, I have to admit that I cannot provide a good plan for achieving this at the moment. @simonharrer Can you perhaps shed some light on how we should proceed here? For now, I think it is ok, if we just try to not to add too many new dependencies between logic and preferences. #1593 is still a step in the right direction, I think, because it decouples calls to Globals, which is also something we need to do. The coupling to JabRefPreferences is still there, but we should take one step at a time.

[simonharrer]

:+1: for moving the event package to the model.

I would create a class with all the standard field names, and put this in the model. Then, this could be used within the model as well.

Regarding preferences: I think, too, that this should be an evolutionary process with the ultimate goal of having no preferences in there. I do not have a plan that fits all situations. What would be best:

• GUI calls Method from Logic, and passes the preference values as parameters.
• If there are many parameters, create a class.
• As the logic should not write preferences, these classes can be made immutable.
• If the preferences influence the state of the logic somewhere, and there is not yet a nice API available, we have a problem. In any case, we need to make this explicit that there is a dependency. Either we pass the preference value for each call, or we store it somewhere and need to update it whenever it changes, which may update even more stuff. We can solve this using the observer pattern so that in case of any preference changes, the state of the logic will change as well. If only one or two methods are touched, I would pass the parameter, but if this would touch a lot more methods, we should use the observer pattern instead.

[lenhard]

There is one more thing I am wondering about: How should the GUI access preferences? Should it directly read preferences from JabRefPreferences, i.e.:

String defaultLabelPattern = prefs.get(JabRefPreferences.DEFAULT_LABEL_PATTERN);

Or should it reuse more specific preferences classes that we introduce at various places, i.e.:

LabelPatternPreferences labelPrefs = LabelPatternPreferences.fromPrefs(prefs);
String defaultLabelPattern = labelPrefs.getDefaultLabelPattern();

@simonharrer, @tobiasdiez, @oscargus : What do you think? At the moment the GUI does everything in the first style. Should one of the two be preferred? Or does it not matter? This is relevant for some of the refactoring work that Oscar is currently doing.

[oscargus]

I actually didn't understand earlier that the preferences are written to different locations as is discussed in #1626. I thought it was enough to have separate keys and everything was fine... This is then clearly relevant for moving/injecting preferences from/in logic. (Or am I missing something more?)

I'd say that the GUI code should access Globals.prefs and the logic code use a special class. However, sometimes it is convenient to use the same class for the GUI. For example, the OO/LO connection has a very distinct set of preferences so one might just as well only work with those. In my recent code I think I mix a bit. Try to use a special preference class, but where it is simpler, I go with Globals.prefs.

[matthiasgeiger]

The idea of the specific preferences was to avoid cluttered method signatures, right?
I.e., doTheWork(SomePreferences withThisPrefs) instead of doTheWork(String pref1, boolean pref2, int pref3, ...) especially in the case that preferences are used during initialising an object (which, however should be avoided to simplify pref changes :wink:).

If we stick to those "specific preferences" and do not use this only as an intermediate step to methods with explicit parameters, I would use the "specific preferences" in the UI, too.
Hiding the prefs key and using the method for a subset of the global preferences seems to be more consistent and simplifies changing the keys a bit (though with current IDEs this is not a real issue ;-))

[lenhard]

Since we have a consensus on the high-level architecture, I now documented it in https://github.com/JabRef/jabref/wiki/High-Level-Documentation

Therefore, I am closing this issue. It can be reopened if necessary and we can also discuss more specific architectural things in new issues.