Skip to content

Conversation

tobiasdiez
Copy link
Member

Moves code from JabRef to own class and introduces new class encapsulating the proxy preferences.

@tobiasdiez tobiasdiez added component: cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jan 9, 2016
@oscargus
Copy link
Contributor

In the spirit of moving things from JabRefPreferences, wouldn't it make sense to let the ProxyPreferences class access the preferences directly?

So: ProxyPreferences proxyPrefs = new ProxyPreferences() and proxyPrefs.save() (possibly with a JabRefPreferences as an argument).

@tobiasdiez
Copy link
Member Author

Yes you are right, this would also be a possibility.

What I aim at in the end is to reduce the public interface of the JabRefPreferences class to like 20 methods which store/retrieve only complex types. In this way also the serialization is hidden in the class and the caller doesn't have to worry how to convert to string/boolean etc. Then it would be also very simple to replace the whole class with something completely different, for example writing the preferences as a json file.

@tobiasdiez
Copy link
Member Author

I now moved some code from the JabRefPreferences class to the new ProxyPreferences class as @oscargus suggested. Looks indeed cleaner.

@tobiasdiez tobiasdiez mentioned this pull request Jan 19, 2016
@simonharrer
Copy link
Contributor

  • no state in ProxyPreferences
  • IDEA: logic and model should not know JabRefPreferences
  • JabRefPreferences must not know ProxyPreferences; defaults and keys in JabRefPrefences

Moves code from `JabRef` to own class and introduces new class
encapsulating the proxy preferences.
@tobiasdiez
Copy link
Member Author

Done & rebased.

simonharrer added a commit that referenced this pull request Jan 26, 2016
Refactor proxy registration and preferences
@simonharrer simonharrer merged commit df99ed8 into JabRef:master Jan 26, 2016
@tobiasdiez tobiasdiez deleted the proxyNew branch January 26, 2016 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants