se-edu / addressbook-level3

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
28 stars 426 forks source link

Reduce code duplication in build.gradle #195

Closed Eclipse-Dominator closed 1 year ago

Eclipse-Dominator commented 1 year ago

Currently, dependencies import for JavaFX is as below:

    implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'win'
    implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'mac'
    implementation group: 'org.openjfx', name: 'javafx-base', version: javaFxVersion, classifier: 'linux'
    implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'win'
    implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'mac'
    implementation group: 'org.openjfx', name: 'javafx-controls', version: javaFxVersion, classifier: 'linux'
    implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'win'
    implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'mac'
    implementation group: 'org.openjfx', name: 'javafx-fxml', version: javaFxVersion, classifier: 'linux'
    implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'win'
    implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'mac'
    implementation group: 'org.openjfx', name: 'javafx-graphics', version: javaFxVersion, classifier: 'linux'

Importing dependencies for JavaFX in build.gradle is highly repetitive. This repetition makes it difficult to read, modify, and edit modules if necessary. Currently, for each JavaFX module, the dependencies for 'win', 'mac' and 'linux' are individually added. To address this issue, it would be more efficient to use a nested for loop to handle these imports.

By implementing a nested for loop, we can reduce code duplication and improve the ease of modifying modules and platforms. Simplifying the import process for JavaFX dependencies will enhance overall code readability and maintainability.

Therefore, let's reduce code duplication by implementing a nested for loop to handle the import of JavaFX dependencies in build.gradle.

canihasreview[bot] commented 1 year ago

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

codecov[bot] commented 1 year ago

Codecov Report

Merging #195 (fb1234c) into master (9356ae9) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #195   +/-   ##
=========================================
  Coverage     73.61%   73.61%           
  Complexity      420      420           
=========================================
  Files            71       71           
  Lines          1285     1285           
  Branches        126      126           
=========================================
  Hits            946      946           
  Misses          307      307           
  Partials         32       32           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

canihasreview[bot] commented 1 year ago

v1

@Eclipse-Dominator submitted v1 for review.

(:books: Archive)

Checkout this PR version locally ```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/195/1/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.
Eclipse-Dominator commented 1 year ago

Do I need to create an issue for this PR?

damithc commented 1 year ago

Do I need to create an issue for this PR?

@Eclipse-Dominator Not strictly necessary.

damithc commented 1 year ago

As discussed, let's not do this for now, in the interest of keeping the build.gradle file structure simple. We can reopen this if we decide to go ahead with this in the future. Thanks for the work @Eclipse-Dominator and for inputs @linustws