Repository: iluwatar/java-design-patterns
Issue: SonarCloud reports issues
Labels: type: bug
info: good first issue
type: refactoring
epic: code quality

The project is using SonarCloud static code analysis. The latest results are showing that it has found several blockers and critical severity code smells (major, minor, and info level we ignore). In this task, those are fixed or marked as false positives. To mark false positives, SuppressWarnings annotation should be used in code as explained in the linked documentation.

Links:
[Blocker and critical severity code smells in SonarCloud](https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&resolved=false&severities=BLOCKER%2CCRITICAL&types=CODE_SMELL)
[Sonar false positives](https://docs.sonarqube.org/latest/analysis/languages/java/#JavaFAQ-HowtoremoveFalse-Positiveissues)

***

Note: When Submitting a PR please provide a **hyperlinked** list of all the issues that you are issuing a fix for. [_See this example_](https://github.com/iluwatar/java-design-patterns/pull/1833#issue-1015516036) 


Issue comments:
Can I take this issue?
Sure @ykayacan 
This issue is free for taking again.
I would like to take this issue.
Through out checking the files causing issues on SonarCloud, I've encountered test cases that are not properly named to the functionality that they are to be tested against. 

Example`

In-correct naming convention of a test method.
> `  @Test
>   void test() throws Exception {
>     App.main(new String[]{});
>   }`

Correct naming convention of a test method.

> `@Test
>   public void shouldExecuteAppWithoutException() {
>     assertDoesNotThrow(() -> App.main(null));
>   }`

A test method should describe what its testing directly in the naming, I will rename the test cases that I find throughout the refactoring process to match proper convention rules, if the test case is understandable as to what it's testing, but I would encourage that these test cases be looked at and properly named for other contributors making changes to certain files that the test cases test against, otherwise it causes a lot of confusion as to whether said contributor is testing the right thing.
Ok @ToxicDreamz, assigned to you. I agree with your comment regarding test naming.
I've already finished with the issue. 

Request to run another SonarCloud report after merging, to see what else remains, and whether or not they can be modified so issues do not come up again.
13 blockers and 24 criticals remaining: https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&resolved=false&severities=BLOCKER%2CCRITICAL
`HotelDaoImpl.java` seems to be one of the hotspots we should fix: https://sonarcloud.io/component_measures?id=iluwatar_java-design-patterns&selected=iluwatar_java-design-patterns%3Atransaction-script%2Fsrc%2Fmain%2Fjava%2Fcom%2Filuwatar%2Ftransactionscript%2FHotelDaoImpl.java&view=list
@iluwatar happy to be back again :)
Can I help with this issue?
I will resolve the report:

https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AW5RukINqcCiTG-gJi15&resolved=false&severities=CRITICAL
Ok, assigned @anuragagarwal561994 and @daniel-augusto. Thanks guys!
@iluwatar Can I help with this issue?
Sure if @anuragagarwal561994 is no longer working on it?
No @iluwatar I didn't get time to look into this, you can re-assign
@iluwatar I will start working on it 
No problem @anuragagarwal561994 
@kanwarpreet25 you're good to go
@iluwatar can I help with this and coordinate with @kanwarpreet25 @daniel-augusto ?
@kanwarpreet25 has no public commits since Jul 28, 2019, [f7e22a1](https://github.com/iluwatar/java-design-patterns/commit/f7e22a1cf6f7664e8790cc5c76285adbcc6ab19d)
@daniel-augusto has no public commits to this repo since Oct 11, 2020 [3a72abb](https://github.com/daniel-augusto/java-design-patterns/pull/1/commits/3a72abba25535b9f3bd90ac71c6cc77c530039d3)

Are you guys working on this know? 
Thanks for helping out @joseosuna-engineer. I'm looking forward to your pull request!
Thank you. I'm going to begin with this.
@iluwatar @ohbus 
I have added this project to my own CircleCI and SonarCloud accounts to run SonarCloud before create a pull request.

**My accounts:**
![image](https://user-images.githubusercontent.com/2281302/113358032-669e9100-931b-11eb-8d05-5b3dbf3c0b7b.png)


I **changed two config files**, :scream: **.circleci/config.yml** and **pom.xml** (my fork, another branch) 
I'm sure @iluwatar @ohbus that we can find a better approach using environment vars or some characteristic of 
CircleCI 2.1 [Youtube](https://www.youtube.com/watch?v=etXD9sCFxIU)

My strategy is: 
- to use this branch **A** only in my fork.
- to create another branch **B** to commit my code.
- to merge **B** to **A** and run CircleCI + SonarCloud
- when code is okay create a pull request from **B** (that has original config files) to master

![image](https://user-images.githubusercontent.com/2281302/113358878-fbee5500-931c-11eb-8cbf-caeeca0a0d52.png)

![image](https://user-images.githubusercontent.com/2281302/113358940-1c1e1400-931d-11eb-940f-a744b88f4a75.png)





I think the general approach presented above is going to work, but as pointed out there are chances to streamline the workflow. @joseosuna-engineer feel free to create another issue where you point out the exact spots we should change.
@iluwatar I have created issue [1697](https://github.com/iluwatar/java-design-patterns/issues/1697)
> 
> 
> The project is using SonarCloud static code analysis. The latest results are showing that it has found several blocker and critical severity code smells (major, minor and info level we ignore). In this task those are fixed or marked as false positives. To mark false positives, SuppressWarnings annotation should be used in code as explained in the linked documentation.
> 
> Links:
> [Blocker and critical severity code smells in SonarCloud](https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&resolved=false&severities=BLOCKER%2CCRITICAL&types=CODE_SMELL)
> [Sonar false positives](https://docs.sonarqube.org/latest/analysis/languages/java/#JavaFAQ-HowtoremoveFalse-Positiveissues)

@iluwatar I want to ask you some things:
1.- This issue (1012) is not about all sonarcloud issues. is it?
2.- major, minor and info issues are not included here
3.- it is only about (blocker and critical) + code smell issues

![image](https://user-images.githubusercontent.com/2281302/113944874-3ea7a580-97d3-11eb-8c10-097e2e03a933.png)

4.- Do you want I add **@java.lang.SuppressWarnings("squid:some-number")** annotation for **all** in question number 3? or Do I have to do something like [ykayacan](https://github.com/ykayacan/java-design-patterns/commit/d5ed1a1f22467d396c1f61f4ac18ded662591ccb): suppress false positives (test by example) and fix number 3 issues?

@joseosuna-engineer 

1. No, it's about blocker and critical level issues only
2. Yes
3. Yes
4. For false positives we can suppress the warnings as you suggested

@iluwatar Thank you! 
Can I help to fix this issue? Has anyone solved this one? https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AW3G0WaAwB6UiZzQNqwC&resolved=false&severities=BLOCKER
@samuelpsouza Yes. Feel free to colaborate. @iluwatar I'm so sorry. I'm studying for a couple of certification test.
> @samuelpsouza Yes. Feel free to collaborate. @iluwatar I'm so sorry. I'm studying for a couple of certification tests.

Please @joseosuna-engineer, no need for an apology here.

You can mention a timeline for when you will be able to make the contributions or let us know in [Gitter](https://gitter.im/iluwatar/java-design-patterns) about the same as well.

And all the very best for your examinations! May you ace them ⭐ 
> Can I help to fix this issue? Has anyone solved this one? https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AW3G0WaAwB6UiZzQNqwC&resolved=false&severities=BLOCKER

Thank you so much for your interest in our project @samuelpsouza through #1784
Raised a PR for the following two Critical Code Smells: [#1831](https://github.com/iluwatar/java-design-patterns/pull/1831)

1. https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AXhD5LIoXfseSECKkezl&resolved=false&severities=CRITICAL&types=CODE_SMELL
2. https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AXhD5LIoXfseSECKkezm&resolved=false&severities=CRITICAL&types=CODE_SMELL

First time contributing to open source software. Please review and provide your feedback.
> Raised a PR for the following two Critical Code Smells: [#1831](https://github.com/iluwatar/java-design-patterns/pull/1831)
> 
> 1. https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AXhD5LIoXfseSECKkezl&resolved=false&severities=CRITICAL&types=CODE_SMELL
> 2. https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AXhD5LIoXfseSECKkezm&resolved=false&severities=CRITICAL&types=CODE_SMELL
> 
> First time contributing to open source software. Please review and provide your feedback.

Please also review https://github.com/iluwatar/java-design-patterns/pull/1833 and https://github.com/iluwatar/java-design-patterns/pull/1832
I would like to work on this issue and resolve some of the code smells/ critical issues in Sonar. Can I take this up? @iluwatar
Sure @parulagg27, go ahead
We need assistance to resolve the remaining blocker and critical level SonarCloud issues
I have resolved 1 blocker & 5 critical Sonar issues using [this PR](https://github.com/iluwatar/java-design-patterns/pull/1899). Kindly review and merge.
<img width="1730" alt="Screenshot 2021-10-30 at 11 27 15 PM" src="https://user-images.githubusercontent.com/18613127/139554128-d1b1d1f6-0d95-4663-b3bf-096aa31b8b26.png">
<img width="1728" alt="Screenshot 2021-10-30 at 11 26 50 PM" src="https://user-images.githubusercontent.com/18613127/139554131-468ada1a-d34c-4cb9-8c71-2a77a3fe1195.png">



- **Resolved** 1 blocker & 5 critical Sonar issues 

- **Old sonar report** :- [here](https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&resolved=false&severities=BLOCKER&types=CODE_SMELL)

- **Code Cov**: Added an extensive test cases for Commander pattern.

- **PR Quality Gate** is also [green](https://sonarcloud.io/summary/new_code?id=iluwatar_java-design-patterns&pullRequest=1899)

@iluwatar  @ohbus 
Kindly review and merge [this PR](https://github.com/iluwatar/java-design-patterns/pull/1899)
hey can i work on this issue :)
Ok, go ahead @ShivanshCharak 
Raised a pr for the following Critical code smell
https://github.com/iluwatar/java-design-patterns/pull/1960

please also review
a minor code smell
https://github.com/iluwatar/java-design-patterns/pull/1959#issue-1118731779
Can I help? 
Ok @S7sRuss, thanks for the help

Pull request review comments:

Patch that resolved the issue:
diff --git a/factory-method/src/main/java/com/iluwatar/factory/method/App.java b/factory-method/src/main/java/com/iluwatar/factory/method/App.java
index b76a918d1876..d903ee7c7fee 100644
--- a/factory-method/src/main/java/com/iluwatar/factory/method/App.java
+++ b/factory-method/src/main/java/com/iluwatar/factory/method/App.java
@@ -41,6 +41,8 @@
 @Slf4j
 public class App {
 
+  private static final String MANUFACTURED = "{} manufactured {}";
+
   /**
    * Program entry point.
    * @param args command line args
@@ -49,14 +51,14 @@ public static void main(String[] args) {
 
     Blacksmith blacksmith = new OrcBlacksmith();
     Weapon weapon = blacksmith.manufactureWeapon(WeaponType.SPEAR);
-    LOGGER.info("{} manufactured {}", blacksmith, weapon);
+    LOGGER.info(MANUFACTURED, blacksmith, weapon);
     weapon = blacksmith.manufactureWeapon(WeaponType.AXE);
-    LOGGER.info("{} manufactured {}", blacksmith, weapon);
+    LOGGER.info(MANUFACTURED, blacksmith, weapon);
 
     blacksmith = new ElfBlacksmith();
     weapon = blacksmith.manufactureWeapon(WeaponType.SPEAR);
-    LOGGER.info("{} manufactured {}", blacksmith, weapon);
+    LOGGER.info(MANUFACTURED, blacksmith, weapon);
     weapon = blacksmith.manufactureWeapon(WeaponType.AXE);
-    LOGGER.info("{} manufactured {}", blacksmith, weapon);
+    LOGGER.info(MANUFACTURED, blacksmith, weapon);
   }
 }


Test patch: