Skip to content

Make Group(Node)TreeViewModel more OO#9978

Merged
Siedlerchr merged 1 commit into
mainfrom
more-oo
Jun 7, 2023
Merged

Make Group(Node)TreeViewModel more OO#9978
Siedlerchr merged 1 commit into
mainfrom
more-oo

Conversation

@koppor

@koppor koppor commented Jun 6, 2023

Copy link
Copy Markdown
Member

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 dev: 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
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into controlsfx-tags
3512118 
* 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

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@calixtus calixtus calixtus approved these changes

Assignees

No one assigned

Labels

dev: code-quality Issues related to code or architecture decisions status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@koppor @Siedlerchr @calixtus