Merge pull request #8 from BME-MIT-IET/2-manuális-kód-átvizsgálás-sta…

…tikus-analízis

2 manuális kód átvizsgálás statikus analízis

docs/Static_analysis.md

@@ -0,0 +1,50 @@
+# Manuális kód átvizsgálás, statikus analízis
+
+## A feladat leírása
+A feladatot a következő lépések mentén valósítottuk meg:
+- Hibák kigyűjtése SonarLint és ErrorLens segítségével
+- Hibák szűrése fontosság szerint
+- A kiszűrt és azonosított híbák kijavítása, változtatások publikálása
+## Feladatmegosztás
+A feladat végzése közben, mivel ketten egy időben csináltuk, folyamatosan beszéltünk egymással egy Discord hívásban. Ennek következményeként nagyon hatékonyan tudtuk a munkát felosztani, a feladattal párhuzamosan haladni és az esetleges merge-conflictokat elkerülni. Általánosságban egy class-t egy ember javított ki, ha elakadt a másiktól kért segítséget.
+## Hibák kigyűjtése
+A VSCode SonarLint és ErrorLens bővítményeivel először is átvizsgáltuk a kódbázist és megnéztük milyen hibákat talál a statikus analízis eszköz. Ezután kigyűjtöttük azokat, amelyek kijavítása nem triviális. Itt a csapattal közösen egyeztünk meg a kijavítandó hibákban. A statikus analízis futtatása során a következő hibákat vettük figyelembe:
+- Save and reuse random
+- Refactor monster class
+- Refactor method to reduce complexity
+- Remove code duplications
+- és más kisebb javítások (olvashatóság, formai szabványok követése, szemantikai és logikai javítások)
+
+Egy SonarLint és ErrorLens bővítményekkel megnyitott file egy sora ami hibás:
+![File with SonarLint and ErrorLens](./images/SonarLintErrorLens.png)
+
+A továbbiakban a fentebb említett hibákat fogjuk részletesebben bemutatni.
+
+## Save and reuse random
+A hibát azért választottuk, mert a hiba kijavításának mellőzése komoly biztonsági hibákat idézhet elő. A hiba a `Cistern.java`, `Pump.java` és `Pipe.java` fájlokban volt. Megoldása egyszerű volt, hiszen csak az újrahasznált változókat kellett osztályonként egyszer példányosítani egy-egy privát tagváltozóként.
+
+## Refactor monster class
+A projektben a `Main.java` osztály isten osztálynak lett jelölve a statikus analízis során, aminek oka az osztály függőségeinek száma volt.
+
+A problémát azonban nem csak a függőségek számának irányából közelítettük meg, hanem inkább a felelősségek szempontjából. Az osztály fő célja a játék kezdőállapotának beállítása és a játékmenet elkezdése, azonban az említett osztály végezte az elemek kirajzolását is.
+
+A hiba orvoslásának érdekében létrehoztunk egy `Renderer.java` osztályt, ahova átkerült a kirajzoláshoz szükséges funkcionalítás. Habár így a `Main` class-ban eggyel több függőséget lett, így is sikerült annak számát összességében csökkenteni, ezzel is biztosítva az olvashatóságot és a SOLID elvek betartását.
+
+## Refactor method to reduce complexity
+A következő classok következő metódusainak komplexitását tudtuk csökkenteni:
+- `MyFrame.java` -> drawLines method
+- `Active.java` -> PipeRelocation method
+- `GraphicsPipe.java` -> update method
+- `GameKeyListener.java` -> keyReleased method
+- `Pump.java` -> Insert method
+A metódusok komplexitását általában egyszerű refaktorálással tudtuk csökkenteni, viszont néhány helyen át kellett írnunk a logikát is. Emellett itt még több más, a SonarLint által nem jelzett logikai hibát javítottunk ki.
+
+## Remove code duplications
+A `Pipe.java` ChangeState és AddPoint metódusait refaktoráltuk, hogy ne legyen benne kódduplikáció. Ezt mindkét metódusban a logika átírásával oldottuk meg.
+
+## Kisebb javítások
+Ezek a javítások általában nem voltak elég nagyméretűek, komplexek és fontosak, hogy külön mindegyikre kitérjünk. Az itt elvégzett javítások között van például: a nem használt importok kitörlése, behúzás javítása, kód olvashatóbbá tétele a kód formájának megváltoztatásával. Ezek a változtatások is javítottak a kód és a projekt minőségén.
+
+## Összegzés, tanulságok
+
+A feladat elvégzésével határozottan sikerült a kód minőségén és olvashatóságán javítani, ezzel bizonyítva a statikus analízis módszerek használatának fontosságát. A feladat során betekintést kaphattunk komplexebb problémák csoportos javításába és a csapatmunka mikéntjébe.

src/main/java/Active.java

@@ -30,48 +30,47 @@ public void Swap(Passive p1,Passive p2)
 	 * @param toEndPoint hányadik kimenetére
 	 * A komponens amelybe be kell kötni a csövet.
 	 */
-	public boolean PipeRelocation(int endPoint,Active to,int toEndPoint)
-	{
-		if(to !=null) {
+	public boolean PipeRelocation(int endPoint,Active to,int toEndPoint){
+		if(to == null) {
+			return false;
+		}
 
-			if (PassiveComponents[endPoint].ActiveComponents[0]!= null && PassiveComponents[endPoint].ActiveComponents[0].equals(this)) //Megnezzuk hogy ez az aktiv a keresett passzivnak hanyadik eleme.
-			{
+		if (PassiveComponents[endPoint].ActiveComponents[0]!= null && PassiveComponents[endPoint].ActiveComponents[0].equals(this)) //Megnezzuk hogy ez az aktiv a keresett passzivnak hanyadik eleme.
+		{
 
-				if(PassiveComponents[endPoint].ActiveComponents[1]!=null){
-					try{
+			if(PassiveComponents[endPoint].ActiveComponents[1]!=null){
+				try{
 
-						PassiveComponents[endPoint].ActiveComponents[1].Swap(PassiveComponents[endPoint], null);
+					PassiveComponents[endPoint].ActiveComponents[1].Swap(PassiveComponents[endPoint], null);
 
-					}
-					catch (Exception e ){
-						e.printStackTrace();
-					}
 				}
-
-				PassiveComponents[endPoint].ActiveComponents[1] = to;//0
-
+				catch (Exception e ){
+					e.printStackTrace();
+				}
 			}
 
-			else if (PassiveComponents[endPoint].ActiveComponents[1]!= null&& PassiveComponents[endPoint].ActiveComponents[1].equals(this))
-			{
+			PassiveComponents[endPoint].ActiveComponents[1] = to;//0
 
-				if(PassiveComponents[endPoint].ActiveComponents[0]!=null){
+		}
 
-					PassiveComponents[endPoint].ActiveComponents[0].Swap(PassiveComponents[endPoint], null);
+		else if (PassiveComponents[endPoint].ActiveComponents[1]!= null&& PassiveComponents[endPoint].ActiveComponents[1].equals(this))
+		{
 
-				}
+			if(PassiveComponents[endPoint].ActiveComponents[0]!=null){
 
-				PassiveComponents[endPoint].ActiveComponents[0] = to;//0
+				PassiveComponents[endPoint].ActiveComponents[0].Swap(PassiveComponents[endPoint], null);
 
 			}
-			if(to.PassiveComponents[toEndPoint]==null)
-				to.PassiveComponents[toEndPoint]= PassiveComponents[endPoint];
-			else {
-				to.Swap(to.PassiveComponents[toEndPoint], PassiveComponents[endPoint]);
-			}
-			return true;
+
+			PassiveComponents[endPoint].ActiveComponents[0] = to;//0
+
 		}
-		return  false;
+		if(to.PassiveComponents[toEndPoint]==null)
+			to.PassiveComponents[toEndPoint]= PassiveComponents[endPoint];
+		else {
+			to.Swap(to.PassiveComponents[toEndPoint], PassiveComponents[endPoint]);
+		}
+		return true;
 	}
 
 	/**
@@ -112,5 +111,7 @@ public int getNeighbourIndex(Passive p)
 	public void setNeighbours(int i, Passive p){
 		PassiveComponents[i]=p;
 	}
-	public  void MoveWater(SComponent c){}
+	public  void MoveWater(SComponent c){
+		//only special Active components are capable of moving water.
+	}
 }

src/main/java/Cistern.java

@@ -6,6 +6,7 @@ public class Cistern extends Active implements Notifiable {
 	private Pump Pump; //Tárolja a ciszternánál lévő generált pumpát
 	private int generate; //Ez majd egy random lesz a gyakorlatban, és csak a Notify-on belül fog létezni [TESZTELÉSHEZ]
 	private boolean randomizable = true;
+	private Random r = new Random();
 	private  int waterCounter;
 	//Konstruktor
 	public Cistern(String s,int X, int Y)
@@ -127,7 +128,6 @@ public int AddPoint(){
 	public void Notify()
 	{
 		if(randomizable) {
-			Random r = new Random();
 			generate=r.nextInt(50);