Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Group(Node)TreeViewModel more OO #9978

Merged
merged 1 commit into from
Jun 7, 2023
Merged

Make Group(Node)TreeViewModel more OO #9978

merged 1 commit into from
Jun 7, 2023

Conversation

koppor
Copy link
Member

@koppor koppor commented Jun 6, 2023

Follow-up to #9955

  • Imperative style: X.doSomething(y) (static or pseudo-static method in X)
  • OO style: y.doSomething()

I rewrote GroupNodeViewModel to be more OO.

More Java 20 would have been following code, but I would had to enable preview features which I did not dare:

    public boolean isEditable() {
        AbstractGroup group = groupNode.getGroup();
        switch (group) {
            case AllEntriesGroup g:
                return false;
            case AutomaticKeywordGroup g:
                return false;
            case AutomaticPersonsGroup g:
                return false;
            case TexGroup g:
                return false;
            case ExplicitGroup g:
                return true;
            case SearchGroup g:
                return true;
            default:
                assert group instanceof KeywordGroup;
                // KeywordGroup (which its children  LastNameGroup, RegexKeywordGroup, WordKeywordGroup)
                return groupNode.getParent()
                         .map(parent -> parent.getGroup())
                         .map(groupParent -> !(groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup))
                         .orElse(false);
        }
    }

Moreover, there is no chaining of classes (e.g., `case AllEntriesGroup | AutomaticKeywordGroup | AutomaticPersonsGroup g)

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@koppor koppor added type: code-quality Issues related to code or architecture decisions status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jun 6, 2023
@Siedlerchr Siedlerchr merged commit 7d64663 into main Jun 7, 2023
@Siedlerchr Siedlerchr deleted the more-oo branch June 7, 2023 17:00
Siedlerchr added a commit that referenced this pull request Jan 1, 2024
* upstream/main: (68 commits)
  Fix issue 9863 - Change Tab selection order (#9907)
  New Crowdin updates (#9994)
  Enable editing typo with double clicking on field name in custom entry type (#9977)
  Remove "Field name:" from localization - we already have "Field name" (#9991)
  Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler (#9989)
  Use environment variables for hostname detection (#9910)
  Add cleanup activity for URL field (#9970)
  Fix freezing when fetching IBSN and no results are found (#9987)
  Make Group(Node)TreeViewModel more OO (#9978)
  Fix container for group item count still visible if display count is off  (#9980)
  Fix paste of BibTeX data (#9985)
  Bring back SimplifyBoolean* and UnnecessaryParantheses (and refine guide) (#9981)
  Add new menu entry to remove braces from selection, aka unprotect it. (#9968)
  User specific comment (#9727)
  Bump org.openrewrite.recipe:rewrite-recipe-bom from 1.19.3 to 2.0.0 (#9975)
  Fix typos
  Remove non-needed link
  Fix typos
  Enable RemoveJavaDocAuthorTag (#9973)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9974)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants