Merge pull request #13 from BME-MIT-IET/Sonar-Fixes

Sonar fixes

.github/workflows/build.yml

@@ -4,6 +4,7 @@ on:
     branches:
       - main
       - Manuális-és-statikus-ellenőrzés
+      - Sonar-Fixes
   pull_request:
     types: [opened, synchronize, reopened]
 jobs:

.gitignore

@@ -12,5 +12,6 @@
 *.OUT
 /bin/
 .idea/
+.idea/*
 out/production/iet-hf-2024-tizenhat_tonna/palya.txt
 /target

Sivatagi vizhalozatok uzemeltetese a gyakorlatban 2/src/Controll/Menu.java

@@ -16,7 +16,6 @@
 
 
 public class Menu extends JFrame implements ActionListener {
-    public String currentTheme;
     private JButton newGame;
     private JButton exitGame;
     private JButton theme;

Sivatagi vizhalozatok uzemeltetese a gyakorlatban 2/src/Controll/WaterCounter.java

@@ -63,7 +63,7 @@ public void reset(){
     public int getMechanic() { return mechanic; }
 
     public ArrayList<Cistern> getCisterns() { return cisterns; }
-    
+
     public ArrayList<Pipe> getPipes() { return pipes; }
 
     public boolean getEnd() { return end; }

doc/SONAR.md

@@ -0,0 +1,29 @@
+# Statikus és manuális ellenőrzés
+## Eszközök
+* Manuális ellenérzés IntelliJ környeztben
+* SonarLint plugin használatával
+* SonarCloud segítségével
+
+## Tevékenység
+* Első lépés a kódbázis megismerése és futtatása volt.
+* Második lépés a Sonar rendszerrel való összektése a projektnek. Ekkor még nem jelentek meg az issuk a felületen, azt csak később sikerült megoldani a projektstuktúra megváltoztatásával.
+* Következő lépés a néhány feltűnő hiba javítása ami a kódbázis megismerése közben feltűnt.
+* Ezt követően több lépésben a SonarLint segítségével a különböző issukat javítottam. Nem minden került javításra, például a pacakgek nevei, illetve a tervek szerint megvalósított propertyk publikusak maradtak, mivel a getterek és setterek nem valósítanának meg egyéb funkciót a változók beállítása és visszaadásán kívül.
+* A végső lépés a SonarCloud segítségével részletsebb vizsgálat és javítás. A program használt random szám generálást, ami nem egy játék környezetben seed nélkül biztonsági kockázat lenne, de itt elhanyagolható. Ezen kívül a duplikált kódsorok számát is csökkentettem, továbbá olyan issukat is javítottam amit nem jelett a SonarLint.
+
+## Illusztáció
+### Javítások előtt
+
+<p align="center">
+<img src="./images/image.png" width="1280">
+<img src="./images/image-1.png" width="1280">
+<img src="./images/image-2.png" width="1280">
+</p>
+
+### Javítások után
+
+<p align="center">
+<img src="./images/image-3.png" width="1280">
+<img src="./images/image-4.png" width="1280">
+<img src="./images/image-5.png" width="1280">
+</p>

src/main/java/Controll/Menu.java

@@ -1,15 +1,9 @@
 package Controll;
 
+import javax.swing.*;
+import java.awt.*;
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
-import java.awt.Color;
-
-import java.io.FileNotFoundException;
-
-import javax.swing.JFrame;
-import javax.swing.JTextField;
-import javax.swing.JButton;
-import javax.swing.JComboBox;
 
 
 public class Menu extends JFrame implements ActionListener {
@@ -30,7 +24,7 @@ public class Menu extends JFrame implements ActionListener {
     public Menu(String name, String t) {
         currentTheme = t;
         setTitle(name);
-        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
+        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
         setResizable(false);
         setLayout(null);
         setBounds(700, 250, 500, 500);
@@ -52,11 +46,11 @@ public Menu(String name, String t) {
         add(theme);
 
         Integer[] numOfMechanics = {2, 3, 4, 5, 6, 7, 8, 9};
-        mechanics = new JComboBox<Integer>(numOfMechanics);
+        mechanics = new JComboBox<>(numOfMechanics);
         mechanics.setBounds(300, 165, 40, 20);
 
         Integer[] numOfSaboteurs = {2, 3, 4, 5, 6, 7, 8, 9};
-        saboteurs = new JComboBox<Integer>(numOfSaboteurs);
+        saboteurs = new JComboBox<>(numOfSaboteurs);
         saboteurs.setBounds(300, 195, 40, 20);
 
         add(mechanics);
@@ -133,13 +127,10 @@ public void actionPerformed(ActionEvent e) {
                     Controller.commandList.add("addplayer D Sab" + i);
                 }
             }
-            try {
-                Controller.Run(); // egyszer fut le, felépíti a pályát, utána a függvényeit kell majd hívni
-            } catch (FileNotFoundException ex) {
-                throw new RuntimeException(ex);
-            }
+            Controller.Run(); // egyszer fut le, felépíti a pályát, utána a függvényeit kell majd hívni
+
             Controller.create();
-            Controller.SetActivePlayer(Controller.getAllPlayers().get(0));
+            Controller.setActivePlayer(Controller.getAllPlayers().get(0));
             this.dispose();
             ViewGame vg = new ViewGame();
             vg.setBackgroundColor(currentTheme);

src/main/java/Controll/ViewGame.java

@@ -1,32 +1,32 @@
 package Controll;
 
-import Drawing.*;
+import Drawing.Drawable;
+import Drawing.PipeDraw;
 import Enums.Fluid;
-import Fields.*;
-
-import Fields.ActiveFields.*;
+import Fields.Pipe;
 
 import javax.swing.*;
 import java.awt.*;
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 public class ViewGame extends JFrame implements ActionListener {
 
-    private Controller controller;
     private static boolean isChosen = false;
     private JPanel gameBackground;
 
     /**
      * HashMap for the Drawables
      */
-    public static HashMap<Drawable, Object> objectDrawNames = new HashMap<>();
+    public static final Map<Drawable, Object> objectDrawNames = new HashMap<>();
     /**
      * HashMap for the Drawables
      */
-    public static HashMap<Object, Drawable> objectDrawReverseNames = new HashMap<>();
+    public static final Map<Object, Drawable> objectDrawReverseNames = new HashMap<>();
 
 
     public static ViewGame vg;
@@ -58,11 +58,11 @@ public class ViewGame extends JFrame implements ActionListener {
     /**
      * ArrayList for the selected elements
      */
-    static ArrayList<Object> selectSequence = new ArrayList<Object>(); //kiválasztott elemek, kiválasztásuk sorrendjében
+    static final List<Object> selectSequence = new ArrayList<>(); //kiválasztott elemek, kiválasztásuk sorrendjében
     /**
      * HashMap for the buttons
      */
-    public static HashMap<JButton, Drawable> buttonToElement = new HashMap<JButton, Drawable>(); //kiválasztó gomb -> kiválasztott rajz
+    public static final Map<JButton, Drawable> buttonToElement = new HashMap<>(); //kiválasztó gomb -> kiválasztott rajz
 
     /**
      * Max number of rounds
@@ -153,16 +153,16 @@ public static boolean getChosen(){
      */
     public ViewGame() {
         setTitle("Sivatagi vízhálózatok üzemeltetése a gyakorlatban 2");
-        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
+        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
         setResizable(true);
         setBounds(400, 150, 1000, 700);
         setLayout(new BorderLayout());
         gameBackground = new JPanel() {
+            @Override
             protected void paintComponent(Graphics g) {
                 Graphics2D g2d = (Graphics2D)g;
                 g2d.setColor(Color.black);
                 super.paintComponent(g2d);
-                int i = 0;
                 for (Drawable draw : objectDrawReverseNames.values()){
                     if (draw instanceof PipeDraw) {
                         Pipe p = (Pipe)ViewGame.objectDrawNames.get(draw);
@@ -233,18 +233,11 @@ protected void paintComponent(Graphics g) {
         vg = this;
     }
 
-    /**
-     * Sets the controller
-     * @param c
-     */
-    public void setController(Controller c){
-        controller = c;
-    }
-
     /**
      * Paints the objects
      * @param g
      */
+    @Override
     public void paint(Graphics g) {
         super.paint(g);
         Graphics2D g2d = (Graphics2D) g;

src/main/java/Drawing/CisternDraw.java

@@ -1,6 +1,7 @@
 package Drawing;
 
 import Controll.ViewGame;
+
 import javax.swing.*;
 import java.awt.*;
 
@@ -31,6 +32,7 @@ public CisternDraw(int tmpX, int tmpY) {
      * @param panel
      * @param g
      */
+    @Override
     public void Draw(JPanel panel, Graphics2D g) {
         cistern.setBounds(x, y, 50, 50);
         cistern.setBorder(BorderFactory.createLineBorder(Color.blue, 5));

src/main/java/Drawing/PipeDraw.java

@@ -41,8 +41,11 @@ public PipeDraw(int x1, int y1, int x2, int y2) {
         pipeB.addActionListener(ViewGame.selectListener);
     }
     public void setCoords(Drawable pumpFrom, Drawable pumpTo) {
-    	int fromX, toX, fromY, toY;
-
+    	int fromX;
+        int toX;
+        int fromY;
+        int toY;
+
     	fromX = pumpFrom.getX();
         fromY = pumpFrom.getY();
         toX = pumpTo.getX();

src/main/java/Drawing/PumpDraw.java

@@ -31,6 +31,7 @@ public PumpDraw(int tmpX, int tmpY) {
      * @param panel The panel to draw on.
      * @param g The graphics to draw with.
      * */
+    @Override
     public void Draw(JPanel panel, Graphics2D g) {
         pump.setBounds(x, y, 50, 50);
         Pump p = (Pump)ViewGame.objectDrawNames.get(this);

src/main/java/Drawing/SaboteurDraw.java

@@ -9,6 +9,7 @@
 import Fields.Pipe;
 import Players.Player;
 import Players.Saboteur;
+
 import javax.swing.*;
 import java.awt.*;
 import java.util.ArrayList;

src/main/java/Drawing/SpringDraw.java

@@ -1,6 +1,7 @@
 package Drawing;
 
 import Controll.ViewGame;
+
 import javax.swing.*;
 import java.awt.*;
 

src/main/java/Fields/ActiveFields/ActiveFields.java

@@ -16,13 +16,13 @@ public abstract class ActiveFields extends Field {
      */
     private ArrayList<Pipe> pipes  = new ArrayList<>();
     @Override
-    public ArrayList<Field> getNeighborFields(){ return new ArrayList<Field>(pipes);}
+    public ArrayList<Field> getNeighborFields(){ return new ArrayList<>(pipes);}
     /**
      * Getter for pipes. Only for child classes.
      * @return The pipes connected to the active field
      */
     public ArrayList<Pipe> getPipes() {
-        if (pipes.size() == 0) return null;
+        if (pipes.isEmpty()) return new ArrayList<>();
         return pipes;
     }
 

src/main/java/Fields/ActiveFields/Cistern.java

@@ -3,14 +3,16 @@
 import Controll.Controller;
 import Fields.Pipe;
 import Players.Player;
+import StringResource.StringResourceController;
+
 import java.util.ArrayList;
 import java.util.Random;
 
 /**
  * Class for Cistern
  * */
 public class Cistern extends ActiveFields{
-
+    private Random random = new Random();
     /**
      * Last created Pipe. Null if the last pump was just taken.
      */
@@ -30,17 +32,16 @@ public Cistern() {
      */
     @Override
     public void step() {
-        if (getPipes() != null) {
+        if (!getPipes().isEmpty()) {
             for (Pipe pipe : getPipes()) {
                 super.setWater(super.getWater() + pipe.getWater());
             }
         }
         if(createdPipe == null){
-            Random r = new Random();
             if(Controller.isTest()){
                 createdPipe = new Pipe(65);
             }
-            else createdPipe = new Pipe(30+r.nextInt(41));
+            else createdPipe = new Pipe(30+random.nextInt(41));
         }
     }
 
@@ -51,26 +52,15 @@ public void step() {
      * */
     @Override
     public Pump createNewPump(boolean b) {
-        Random r = new Random();
         if(b){
             if(Controller.isTest()){
                 return new Pump(100);
             }
-            else return new Pump(80+r.nextInt(41));
+            else return new Pump(80+random.nextInt(41));
         }
         else return null;
     }
 
-    /**
-     * Method for getting the water from the field.
-     * Prints the amount of water taken.
-     * @return The amount of water in the field.
-     */
-    @Override
-    public int getWater() {
-        return super.getWater();
-    }
-
     /**
      * Method for picking up a (new) pipe from the field.
      * @return The new pipe.
@@ -94,35 +84,16 @@ public Pipe pickUpPipe() {
     @Override
     public String toString() {
         ArrayList<Player> players = this.getPlayers();
-        String playersNames = "null";
-
-        for (int i = 0; i < players.size(); i++) {
-            if(i == 0) playersNames = "";
-            playersNames += Controller.objectReverseNames.get(players.get(i));
-            if (i != players.size() - 1) {
-                playersNames += ", ";
-            }
-        }
-
-
+        String playerBuilder = StringResourceController.stingBuilder(players);
 
         ArrayList<Pipe> pipes = this.getPipes();
-        String pipesNames ="null";
-        if(pipes != null) {
-            for (int i = 0; i < pipes.size(); i++) {
-                if (i == 0) pipesNames = "";
-                pipesNames += Controller.objectReverseNames.get(pipes.get(i));
-                if (i != pipes.size() - 1) {
-                    pipesNames += ", ";
-                }
-            }
-        }
+        String pipeBuilder = StringResourceController.stingBuilder(pipes);
 
           return "name: "+ Controller.objectReverseNames.get(this)
                   + "\noccupied: " + this.isOccupied()
                   + "\nwater: " + getWaterNoChange()
                   + "\nbroken: " + this.isBroken()
-                  + "\nplayers: " + playersNames
-                  + "\npipes: " + pipesNames + "\n";
+                  + "\nplayers: " + playerBuilder
+                  + "\npipes: " + pipeBuilder + "\n";
     }
 }