Skip to content

Conversation

JanFidor
Copy link
Contributor

Add an MVI design pattern (includes pattern itself, tests, README.md and class diagram)

@iluwatar
Copy link
Owner

iluwatar commented Nov 5, 2022

The build is failing. Please investigate @JanFidor

@JanFidor
Copy link
Contributor Author

JanFidor commented Nov 7, 2022

@iluwatar It seems an independent module is causing issues (page-object). My build is failing even on the master branch after syncing the fork. The first error is caused by missing test dependencies, after adding them this line throws an error java: incompatible types: java.util.List<java.lang.Object> cannot be converted to java.util.List<com.gargoylesoftware.htmlunit.html.HtmlAnchor>
I'm not certain if I managed to break git on my part or if it's something else.

@iluwatar
Copy link
Owner

The error from the CI build is Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project model-view-intent: You have 2 Checkstyle violations. @JanFidor

@JanFidor
Copy link
Contributor Author

JanFidor commented Dec 3, 2022

@iluwatar my bad for misinterpreting logs and sorry for the delay, university gave me plenty of work this week, but I'll get right on it now.

@JanFidor
Copy link
Contributor Author

@iluwatar fixed the violations, was a little closer to 60 after I run checkstyle locally, but it checks out now

@JanFidor
Copy link
Contributor Author

Hi, @iluwatar sorry for the long break, but the past couple of weeks have been pretty crazy on my end, but I'll be getting on it now

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 12 Code Smells

98.7% 98.7% Coverage
0.0% 0.0% Duplication

* and displays its updated {@link CalculatorModel}.
*/
@Slf4j
@Data
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjectToString: CalculatorViewModel is final and does not override Object.toString, so converting it to a string will print its identity (e.g. CalculatorViewModel@4488aabb) instead of useful information.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

/**
* SetVariable {@link CalculatorAction}.
*/
@Data
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 7 similar findings have been found in this PR


Var: Non-constant variable missing @var annotation


Suggested change
@Data
@Var @Data

🔎 Expand here to view all instances of this finding
File Path Line Number
model-view-intent/src/main/java/com/iluwatar/model/view/intent/App.java 59
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorViewModel.java 27
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorModel.java 9
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorViewModel.java 70
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 19
model-view-intent/src/main/java/com/iluwatar/model/view/intent/App.java 54
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 71

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

/**
* SetVariable {@link CalculatorAction}.
*/
@Data
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 3 similar findings have been found in this PR


Varifier: Consider using var here to avoid boilerplate.


Suggested change
@Data
var

🔎 Expand here to view all instances of this finding
File Path Line Number
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 19
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorViewModel.java 46
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorModel.java 9

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

/**
* Current state of calculator.
*/
@Data
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 26 similar findings have been found in this PR


UnnecessarilyFullyQualified: This fully qualified name is unambiguous to the compiler if imported.


Suggested change
@Data
Override

🔎 Expand here to view all instances of this finding
File Path Line Number
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 18
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 19
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorModel.java 15
model-view-intent/src/main/java/com/iluwatar/model/view/intent/actions/SetVariableCalculatorAction.java 9
model-view-intent/src/main/java/com/iluwatar/model/view/intent/actions/SetVariableCalculatorAction.java 9
model-view-intent/src/main/java/com/iluwatar/model/view/intent/actions/SetVariableCalculatorAction.java 9
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 19
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 25
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorModel.java 9
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorModel.java 21

Showing 10 of 26 findings. Visit the Lift Web Console to see all.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

*
* @param variable -> value of new calculator model variable.
*/
private void setVariable(final Double variable) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 3 similar findings have been found in this PR


UnnecessaryFinal: Since Java 8, it's been unnecessary to make local variables and parameters final for use in lambdas or anonymous classes. Marking them as final is weakly discouraged, as it adds a fair amount of noise for minimal benefit.


Suggested change
private void setVariable(final Double variable) {
private void setVariable( Double variable) {

🔎 Expand here to view all instances of this finding
File Path Line Number
model-view-intent/src/main/java/com/iluwatar/model/view/intent/App.java 54
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorViewModel.java 27
model-view-intent/src/main/java/com/iluwatar/model/view/intent/CalculatorView.java 71

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@sonatype-lift
Copy link

sonatype-lift bot commented Jan 17, 2023

🛠 Lift Auto-fix

Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.1

# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/iluwatar/java-design-patterns/2177.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff

Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:

curl https://lift.sonatype.com/api/patch/github.com/iluwatar/java-design-patterns/2177.diff | git apply

Once you're satisfied commit and push your changes in your project.

Footnotes

  1. You can preview the patch by opening the patch URL in the browser.

@JanFidor
Copy link
Contributor Author

@iluwatar I made the changes you suggested, I'm not certain what to do about sonatype-lift errors, especially when it comes to Lombok decorators and deleting the final keyword which would cause chackstyle errors (just like in the FSM PR)

@iluwatar
Copy link
Owner

Lift doesn't seem to handle Lombok annotations too well, so I think we can disregard them.

@iluwatar iluwatar merged commit fb86ca1 into iluwatar:master Jan 21, 2023
@iluwatar
Copy link
Owner

Looks good! Thank you for contributing this new pattern 🎉

@all-contributors please add @JanFidor for code

@allcontributors
Copy link
Contributor

@iluwatar

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

@JanFidor already contributed before to code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants