Prośba o ocenę kodu pierwszego programu w języku JAVA

Question

Hej, pracowałem sobie w ramach praktyki nad programem do rozgrzewki mózgu przed nauką kodowania.

Program działa na zasadzie generowania prostych działań matematycznych, które użytkownik może wybrać : dodawanie, odejmowanie, mnożenie i dzielenie(do pełnej wartości). Za każdym razem użytkownik ma 3 próby.

Byłbym wdzięczny, jeśli ktoś mógłby rzucić na to okiem i wytknąć mi złe praktyki. Chciałbym od samego początku złapać dobre nawyki.

public interface IDziałania {

	public int dodawanie();
	public int odejmowanie();
	public int mnożenie();
	public int dzielenie();
}

public class Procedura extends Losowanie implements IDziałania{
	

	public int dodawanie() {
		int dod = super.getL1() + super.getL2();

		return dod;
	}

	public int odejmowanie() {
		int ode = super.getL1() - super.getL2();

		return ode;
	}

	public int mnożenie() {
		int mno = super.getL1() * super.getL2();

		return mno;
	}

	public int dzielenie() {
		int dzie = super.getL1() / super.getL2();

		return dzie;
	}

import java.util.Random;

public class Losowanie{
	Random x = new Random();
	Random y = new Random();

	private int l1 = x.nextInt(99);
	private int l2 = y.nextInt(99);

	public int getL1() {

		if (l1 > 0) {
			return l1;
		} else {
			return l1++;
		}
	}

	public int getL2() {

		if (l2 > 0) {
			return l2;
		} else {
			return l2++;
		}
	}
}

import java.util.InputMismatchException;
import java.util.Scanner;

public class Sprawdzanie {

	public static final int LICZBA_PROB = 3;
	private int zycia = LICZBA_PROB;

	Procedura procedura = new Procedura();

	public void dzialania() {

		System.out.println("Wybierz numer operacji: Dodawanie[1], Odejmowanie[2], Mnożenie[3], Dzielenie[4]");
		Scanner scanner1 = new Scanner(System.in);
		String wybor = scanner1.nextLine();

		int wynik1;

		Scanner wynik = new Scanner(System.in);

		switch (wybor) {

		// DODAWANIE
		case "1":
			try {
				do {

					System.out.println("Podaj wynik dodawania\t" + procedura.getL1() + "+" + procedura.getL2()
							+ "\tPozostała ilość prób: " + zycia);
					wynik1 = wynik.nextInt();
					--zycia;
					if (zycia == 0) {
						System.out.println("Pozostało " + zycia + " prób. " + "PRZEGRAŁEŚ :(");
						System.exit(0);
					}

				} while (wynik1 != procedura.dodawanie());
				System.out.println("BRAWO!!! :) :) :) ");
			} catch (InputMismatchException ime) {

				System.out.println("Żarty sobie robisz? Podaj LICZBĘ");

			}
			wynik.close();
			break;

		// ODEJMOWANIE
		case "2":

			try {

				do {
					System.out.println("Podaj wynik odejmowania\t" + procedura.getL1() + "-" + procedura.getL2()
							+ "\tPozostała ilość prób: " + zycia);
					wynik1 = wynik.nextInt();
					--zycia;
					if (zycia == 0) {
						System.out.println("Pozostało " + zycia + " prób. " + "PRZEGRAŁEŚ :(");
						System.exit(0);
					}

				} while (wynik1 != procedura.odejmowanie());
				System.out.println("BRAWO!!! :) :) :) ");

			} catch (InputMismatchException ime) {
				System.out.println("Podaj LICZBĘ");
			}
			wynik.close();
			break;

		// MNOŻENIE
		case "3":

			try {

				do {
					System.out.println("Podaj wynik mnożenia\t" + procedura.getL1() + "*" + procedura.getL2()
							+ "\tPozostała ilość prób: " + zycia);
					wynik1 = wynik.nextInt();
					--zycia;
					if (zycia == 0) {
						System.out.println("Pozostało " + zycia + " prób. " + "PRZEGRAŁEŚ :(");
						System.exit(0);
					}

				} while (wynik1 != procedura.mnożenie());
				System.out.println("BRAWO!!! :) :) :) ");

			} catch (InputMismatchException ime) {
				System.out.println("Podaj LICZBĘ");
			}
			wynik.close();
			break;

		// DZIELENIE
		case "4":

			try {

				do {
					System.out.println("Podaj wynik dzielenia\t" + procedura.getL1() + ":" + procedura.getL2()
							+ "\tPozostała ilość prób: " + zycia);
					wynik1 = wynik.nextInt();
					--zycia;
					if (zycia == 0) {
						System.out.println("Pozostało " + zycia + " prób. " + "PRZEGRAŁEŚ :(");
						System.exit(0);
					}

				} while (wynik1 != procedura.dzielenie());
				System.out.println("BRAWO!!! :) :) :) ");
			} catch (InputMismatchException ime) {
				System.out.println("Podaj LICZBĘ");
			}
			wynik.close();
			break;

		default:
			System.out.println("Nieznana komenda");

			
		}
		scanner1.close();
	}
}

public class Gra {
	public static void main(String[] args) {
		Sprawdzanie sprawdzanie = new Sprawdzanie();

		sprawdzanie.dzialania();
	

	}

}

 

Za wszelką krytykę z góry serdecznie dziękuję :)

Comments

Po waszych uwagach przepisałem kod od nowa.Jest dostępny w ostatniej odpowiedzi.

Answer 1

Fajnie, że od początku jesteś nastawiony na dobre praktyki:

• Unikaj języka polskiego w kodzie, a tym bardziej polskich znaków.
• Dla stringa nie korzystaj ze switcha. W Javie porównujemy stringi metodą equals.
• Sprawdzanie::dzialania jest z pewnością za dużą metodą. Da się ją rozbić na mniejsze i bardziej czytelne.
• Dodawanie, odejmowanie itp. to rodzaje operacji. Każda działa inaczej, ale mają jakby wspólne podłoże abstrakcyjne. Może wyodrębnić je do klas, zastosować polimorfizm i jakąś strukturę danych 
• Pisz dokumentacje. Szczególnie ważne jeśli kiedyś Twój kod będzie rozwijany przez inne osoby.
• Unikaj nazewnictwa w stylu l2. To wygląda jak 12. Czystość kodu mierzymy w co to jest / minuta.
• Odradzam korzystanie z return cos++; Jeśli przed returnem cos ma wartość 12, to metoda zwróci właśnie 12, ponieważ post inkrementacja nie zwraca wyniku swojego działania.
• Polecam Ci spróbować korzystać z JUnita. Kod, który testujesz jest pewniejszy, ma mniej bugów i wykonywanie testów podczas tworzenia oprogramowania zapobiega burzeniu tego, co już się zbudowało (chciaż wg SOLID poszczególne komponenty nie powinny mieć na siebie dużego wpływu ). 
• Korzystaj z gita.

Comments

Haha, jestem na tym etapie nauki, że nie rozumiem połowy Twojego komentarza :P: Ale się dowiem! Dzięki serdeczne za obszerną listę wskazówek :)

---

Od czego polecałbyś zacząć rozszerzanie wiedzy ponad czystą Javę? Zamierzałem się brać za Springa i MySql, ale teraz napisałeś mi o tym GIT i Junit. Czas niestety ograniczony, więc jakby trzeba się zdecydować na coś wg. wpływu na dalszy progres.

---

Jak najbardziej idź w stronę Springa skoro interesuje Cię backend. Git tak czy inaczej czego byś się nie uczył to podstawa. A JUnit to duży atut przy zatrudnieniu.

Answer 2

 public class Sprawdzanie
   public void dzialania()

Bardzo długa metoda z komentarzami. Z każdego casa można zrobić przynajmniej osobna metodę prywatna, a z komentarza zrobić nazwę metody.

Nazywanie zmiennych w stylu wynik1, wynik nie jest dobrym pomysłem ponieważ nie widać wyraźniej różnicy co do przeznaczenia tych zmiennych. Trzeba dodatkowo analizować kod żeby wywnioskować do czego, która służy.

Np. w klasie losowanie wystarczy jedna instancja Random, można nawet zapisać to jako static:

private static Random rand = new Random();

private int l1 = rand.nextInt(99);
private int l2 = rand.nextInt(99);

Zastanowić się tez można czy to dziedziczenie po klasie Losowanie jest na pewno konieczne.

Comments

Okej, zanotowałem sobie wszystko i zaraz popracuję nad tym kodem. Dziedziczenie po klasie Losowanie jest rodzajem ćwiczenia wiedzy z tutoriali, żeby ją utrwalić, ale teraz jak się zastanawiam, to rzeczywiście zrobiłem to na siłę.

Dwa pytania mi się nasuwają:

1. Natomiast nie bardzo wiem jak zrobić z casów te metody prywatne, o których mówisz. Da się to zrobić tak, żeby wybór rodzaju działania nadal mógł być determinowany przez input użytkownika?

2. Czy komentarze są czymś złym w kodzie?

---

2. Nadmierne komentarze są złem. Kod powinien się sam komentować poprzez stosowanie dobrych praktyk pisania kodu np. dobre, opisowe nazwy zmiennych, metod, klas; formatowanie kodu (tabulacje, entery, układ metod w klasie (w sensie wizualnym) ) - opis tego wszystkiego i wiele więcej znajdziesz w książce Czysty kod -> https://helion.pl/ksiazki/czysty-kod-podrecznik-dobrego-programisty-robert-c-martin,czykov.htm

A i w internecie pewnie jest pełno artykułów o tych zagadnieniach. Jak wpiszesz programowanie dobre praktyki w wyszukiwarkę to na pewno wyskoczy coś ciekawego.

1. Tak na pierwszy rzut oka. Ze zmiennej int wynik1 (koniecznie zmień nazwę albo tej albo tej drugiej wynik, tym bardziej ze zmienna wynik jest innego typu) zrób zmienną klasową (tak jak zycie) i powinno się już dać, w miarę możliwości, wydzielić metody.

---

Zanotowane. Dziękuję za porady i poświęcony czas :)

Answer 3

Poza tym co już zostało powiedziane to jeszcze klasa Losowanie jest do poprawy. 

Random x = new Random();
    Random y = new Random();
 
    private int l1 = x.nextInt(99);
    private int l2 = y.nextInt(99);

Nie musisz tworzyć dla każdej zmiennej innego Randoma. Możesz śmiało dla obu użyć tego samego. 

private int l1 = x.nextInt(99); 
public int getL1() {
 
        if (l1 > 0) {
            return l1;
        } else {
            return l1++;
        }
    }

Zamiast specjalnie sprawdzać czy przypadkiem nie wylosowano 0 zrób tak :

private int l1 = x.nextInt(99) + 1;  // losowanie od 1 do 99 zamiast od 0 do 98
public int getL1() {
            return l1;
    }

 

Comments

Dzięki! Co do ostatniego punktu, to paradoksalnie początkowo kod miał właśnie to rozwiązanie, ale przekombinowałem.

Answer 4

Po waszych uwagach przepisałem kod od nowa.

Answer 5

public interface ICountable {
	
int addition();
int subtraction();
int multiplication();
int division();

}

public class Runner {

	public static void main(String[] args) {

		Checking answer = new Checking(); 

		Scanner usersChoice = new Scanner(System.in);
		System.out.printf("Wybierz numer operacji:\n1.Dodawnie\n2.Odejmowanie\n3.Mnożenie\n4.Dzielenie");
		int choice = usersChoice.nextInt();

		switch (choice) {
		case 1:
			answer.checkingAddition();
			break;

		case 2:
			answer.checkingSubtraction();
			break;

		case 3:
			answer.checkingMultiplication();
			break;

		case 4:
			answer.checkingDivision();
			break;

		default:
			System.out.println("Nieznana komenda. Proszę podaj numer operacji.");
			System.exit(0);
		}
		usersChoice.close();
	}

}

public class Operations  implements ICountable {
FigureDrawing draw = new FigureDrawing();
	
	
	@Override
	public int addition() {
		int additionResult = draw.getFirstFigure() + draw.getSecondFigure();
		return additionResult;
	}

	@Override
	public int subtraction() {
		int subtractionResult = draw.getFirstFigure() - draw.getSecondFigure();
		return subtractionResult;
	}

	@Override
	public int multiplication() {
		int multiplicationResult = draw.getFirstFigure() * draw.getSecondFigure();
		return multiplicationResult;
	}

	@Override
	public int division() {
		int divisionResult = draw.getFirstFigure() / draw.getSecondFigure();
		return divisionResult;
	}

}

import java.util.Random;

public class FigureDrawing {
	Random draw = new Random();
	
	private int firstFigure = draw.nextInt(99) + 1;
	private int secondFigure = draw.nextInt(99) + 1;
	
	public int getFirstFigure(){
		return firstFigure;
	}
	
	public int getSecondFigure(){
		return secondFigure;
	}

}

import java.util.InputMismatchException;
import java.util.Scanner;

public class Checking {

	Operations operations = new Operations();

	Scanner usersInput = new Scanner(System.in);
	
	public static int MAX_TRIES = 3;
	protected int triesLeft = MAX_TRIES;

	
	protected void checkingAddition() {
		try {
			int usersAnswer;

			do {
				System.out.println("Podaj wynik dodawania:\t" + operations.draw.getFirstFigure() + "+"
						+ operations.draw.getSecondFigure() + "\t Pozostała ilość prób: " + triesLeft);

				usersAnswer = usersInput.nextInt();
				
				--triesLeft;
				if(triesLeft < 1){
					System.out.println("Koniec gry!");
					System.exit(0);
				}

			} while (usersAnswer != operations.addition());
			System.out.println("BRAWO");

		} catch (InputMismatchException ime) {
			System.out.println("Podaj wynik w postaci liczby!");
			usersInput.nextLine();
			checkingAddition();
		}
	}

	
	protected void checkingSubtraction() {
		try {
			int usersAnswer;
			do {
				System.out.println("Podaj wynik odejmowania:\t" + operations.draw.getFirstFigure() + "-"
						+ operations.draw.getSecondFigure() + "\t Pozostała ilość prób: " + triesLeft);

				usersAnswer = usersInput.nextInt();
				
				--triesLeft;
				if(triesLeft < 1){
					System.out.println("Koniec gry!");
					System.exit(0);
				}

			} while (usersAnswer != operations.subtraction());
			System.out.println("BRAWO");
		} catch (InputMismatchException ime) {
			System.out.println("Podaj wynik w postaci liczby!");
			usersInput.nextLine();
			checkingSubtraction();
		}
	}

	
	protected void checkingMultiplication() {
		try {
			int usersAnswer;
			do {
				System.out.println("Podaj wynik mnożenia:\t" + operations.draw.getFirstFigure() + "*"
						+ operations.draw.getSecondFigure() + "\t Pozostała ilość prób: " + triesLeft);

				usersAnswer = usersInput.nextInt();
				
				--triesLeft;
				if(triesLeft < 1){
					System.out.println("Koniec gry!");
					System.exit(0);
				}

			} while (usersAnswer != operations.multiplication());
			System.out.println("BRAWO");
		} catch (InputMismatchException ime) {
			System.out.println("Podaj wynik w postaci liczby!");
			usersInput.nextLine();
			checkingMultiplication();
		}
	}

	
	protected void checkingDivision() {
		try {
			int usersAnswer;
			do {
				System.out.println("Podaj wynik dzielenia:\t" + operations.draw.getFirstFigure() + ":"
						+ operations.draw.getSecondFigure() + "\t Pozostała ilość prób: " + triesLeft);

				usersAnswer = usersInput.nextInt();
				
				--triesLeft;
				if(triesLeft < 1){
					System.out.println("Koniec gry!");
					System.exit(0);
				}

			} while (usersAnswer != operations.division());
			System.out.println("BRAWO");
		} catch (InputMismatchException ime) {
			System.out.println("Podaj wynik w postaci liczby!");
			usersInput.nextLine();
			checkingDivision();
		}
	}
}