Sprawdzenie "czytelnosci" kodu.

0

Hi, z javą mam styczność dosłownie od kilku dni, poza tym ledwie kilkumiesięczne zaplecze w Delphi. Żeby się oswoić ze składnią napisałem program, który powtarza losowania totolotka dopóki nie trafimy głównej nagrody czyli "szóstki" (albo piątki, zależnie jaką liczbę usmieścimy w kodzie). To o co bym prosił to opinia doświadczonych coderów czy kod jest czytelny i czy dałoby się go napisać wydajniej/zamknąć w mniejszej ilości linijek (a zapewne się da). Ewentualnie inne rady, tam gdzie uznałem za stosowne umieściłem komentarze, pozdro.

import java.util.Arrays;
import java.util.Scanner;


public class wysw {

    public static void main(String[] args) {
    	
    	
   int n = 50;
int usera[] = new int[6]; //TABLICA PRZECHOWUJACA "SKRESLONE" LICZBY NA KUPONIE
int result[] = new int[6]; //TABLICA PRZECHOWUJACA LICZBY WYLOSOWANE W AKTUALNYM LOSOWANIU
int numbers[] = new int[n]; //TABLICA Z LICZBAMI OD 1 DO 50
int r; //ZMIENNA PRZECHOWUJACA LOSOWA LICZBE
int licznik = 0; //ZMIENNA PRZECHOWUJACA ILOSC TRAFIONYCH LICZB W AKTUALNYM LOSOWANIU
int illos = 0; //LICZNIK LOSOWAN

usera[0] = 8;
usera[1] = 14;
usera[2] = 19;
usera[3] = 27;
usera[4] = 33;
usera[5] = 41;


do{
for (int i = 0; i <numbers.length; i++){
	numbers[i] = i + 1; //NAJZWYKLEJSZE ZAPELNIENIE TABLICY 
}

	licznik = 0;
	n = 50;
	
for (int i = 0; i < 6; i++){
	r = (int) (Math.random() * n);
	result[i] = numbers[r];
	numbers[r] = numbers[n - 1];
	n--;
	System.out.print(result[i] + " "); //WYLOSOWANIE 6 LICZB DO TABLICY RESULT I WRZUCENIE
	                                   //OSTATNIEJ LICZBY W MIEJSCE W TEJ WYLOSOWANEJ  
}



System.out.print("\n");

for (int i = 0; i < 6; i++){
	for (int j = 0; j < 6; j++){
		if (usera[i] == result[j]){
		licznik = licznik + 1; //SPRAWDZENIE CZY COS TRAFILISMY
	}	
	}
}

illos = illos + 1; //ZLICZANIE LOSOWAN

System.out.println("Licznik: " + licznik);
System.out.println("Losowanie nr: " + illos);

}
while(licznik < 6); //JAK CHCEMY POLOWAC NA SZOSTKE TO WPISUJEMY TUTAJ 6, PIATKE 5 ITD

    
}


}
2

Tragiczne wcięcia. Jak nie potrafisz jeszcze samemu to korzystaj z narzędzi, choćby takich online: http://courses.cs.washington.edu/courses/cse341/10au/indent.html

Niepotrzebne komentarze zaciemniają kod zamiast pomóc rozumieć

//NAJZWYKLEJSZE ZAPELNIENIE TABLICY 

Zamiast nadać zmiennej dziwną nazwę i potem tłumaczyć w komentarzu do czego służy, należy po prostu porządnie nazwać zmienną

int usera[] = new int[6]; //TABLICA PRZECHOWUJACA "SKRESLONE" LICZBY NA KUPONIE
int r; //ZMIENNA PRZECHOWUJACA LOSOWA LICZBE
int illos = 0; //LICZNIK LOSOWAN

Tablicę można od razu inicjalizować

int usera[] = {8, 14, 19, 27, 33, 41};
2

Wcięcia bardzo losowe, a nawet często brak...

Wszystko w mainie to zła praktyka. Przydałoby się podzielić aplikację na odpowiednio nazwane metody -> wtedy kod sam mówi, co robi.

Zmniejszanie ilości linijek nie powinno mieć priorytetu nad zwiększeniem czytelności.

Deklaracja zmiennych nie powinna wymagać komentarza. Odpowiednie nazewnictwo powinno wystarczyć. Czym jest n?. Zapis n = 50 nic nie mówi czytającemu.

Dużo by jeszcze można wymieniać, ale lepiej sobie utrwalisz czytając książkę "Czysty Kod" :)

@twonek mnie ubiegł, ale w sumie napisałem coś więcej ;)

0

Dzięki, o to mi chodziło. "Czysty kod" jest następny na mojej liście.

Wszystko w mainie to zła praktyka. Przydałoby się podzielić aplikację na odpowiednio nazwane metody -> wtedy kod sam mówi, co robi.

Tak, wiem. Na początku miałem zamiar to napisać z wykorzystaniem klas i metod, ale jakoś nie miałem pomysłu jak to poumieszczać w metodach. Byłbym wdzięczny za jakieś nakierowanie.

4

To co napisali koledzy wyżej plus:

  1. w twoim programie kluczowa jest liczba 6, jakbyś w przyszłości chciał ja zmienić i losować 7 elementów musiałbyś ją zmieniać w iluś miejscach, dlatego utwórz zmienną iloscLosowanychLiczb = 6;
  2. Zamiast komentarzy sensowne nazwy zmiennych oraz wydzielanie kodu do osobnych metod. Tak żeby jedna metoda byłą odpowiedzialna za jedną rzecz, czyli np. inicjalizujTablicę, losujLiczby, sprawdzCzyWylosowano itp
  3. bez sensu definiujesz wszystkie zmienne na górze, jeśli r jest używana tylko w wąskim fragmencie, definiuj ja tuż przed miejscem pierwszego użycia, podobnie inne zmienne jak licznik i cała reszta.
  4. oczywiście nazwy wszelkie zmiennych metod po angielsku.
1
slayer9 napisał(a):

Tak, wiem. Na początku miałem zamiar to napisać z wykorzystaniem klas i metod, ale jakoś nie miałem pomysłu jak to poumieszczać w metodach. Byłbym wdzięczny za jakieś nakierowanie.

Jeśli w pętli masz więcej niż jedną operację - upakuj to w metodę i odpowiednio nazwij.

Tam gdzie masz kod pozagnieżdżany : for, for, if. Chociażby drugiego fora wsadź do odpowiednio nazwanej metody.

Masz komentarze nazywające co robi dana pętla:

//WYLOSOWANIE 6 LICZB DO TABLICY RESULT I WRZUCENIE
//OSTATNIEJ LICZBY W MIEJSCE W TEJ WYLOSOWANEJ
[...]
//SPRAWDZENIE CZY COS TRAFILISMY

One podpowiadają Ci jak nazwać metody, w których powinieneś umieścić te operacje.

main, a nawet lepiej - metoda uruchamiana w mainie powinna wywołać kilka metod zamiast robić wszystko po sobie:

kupon = LosujLiczbyNaKuponie();
losowanie = WykonajLosowanie();
iloscTrafionych = Porównaj(kupon, losowanie); // to można w pętli robić jak chcesz kilka losowań

Tak by to wyglądało mniej więcej wprost z Twojego kodu. Bo można to podzielić na więcej klas ;) Właściwie kupon, czy losowanie mogą mieć własne klasy. Można też zastosować dziedziczenie. Klasa bazowa po prostu zawiera liczby. Kupon posiada liczby ustalone ręcznie, losowanie posiada metodę losującą liczby.

0

Przerobiłem pętlę pierwszą tak, aby wykorzystywała klasy i metody:

public class kupon{
    int w;
    
    int sortuj(int g){
        for (int i = 0; i <= g; i++){
            w = i;
        }
        return w;
    }
}

public class wysw
{
    public static void main(String[] args)
    {
        kupon[] numbers = new kupon[50];
        
        for (int i = 0; i <numbers.length; i++){
            numbers[i] = new kupon();
            numbers[i].sortuj(i);
            System.out.println(numbers[i].w);
        }

Generalnie po kilku godzinach kombinowania osiągnąłem efekt wypełnienia tablicy używając klas i metod, ale niestety nie wiem co mi to dało ; <

0

Klasa kupon powinna posiadać tablicę lub listę z numerkami ;)
Tworzysz jedną instancję klasy kupon, którą inicjujesz numerkami.
Nazwy klas w Javie powinny być pisane z dużej litery.

Czyli Twój kupon powinieneś używać w taki sposób:

Kupon kupon = new Kupon(tablicaLiczb);

Klasa Losowanie również powinna zawierać tablicę liczb, tylko zamiast podawać jej poszczególne liczby, powinna ona je sobie losować, dlatego powinieneś móc ją używać w ten sposób:

Losowanie losowanie = new Losowanie(iloscLosowanychLiczb);

W tym momencie klasa losowanie powinna zawierać tablicę z wylosowaną odpowiednią ilością liczb (niepowtarzających się!).
Klasa losowanie może mieć metodę, która podaje ilość trafień z kuponu, ta metoda powinna brać Kupon jako argument oraz zwracać int:

int iloscTrafien = losowanie.getMatchesCount(kupon);

Jak wyglądają konstruktory klas oraz dostęp do ich pól, powinieneś móc sobie dośpiewać, bo przecież uczysz się programowania obiektowego, a nie strukturalnego, prawda :) ?

0

Na razie tylko pytanie odnośnie pierwszej klasy (kupon). Doszedłem do czegoś takiego, czy taki tok postępowania miałeś na myśli ?

public class Kupon{
    
    public Kupon(int[] a){
        for (int i = 0; i < a.length; i++){
            a[i] = i;      
        }
    }
    
}

public class Gra
{
    public static void main(String[] args){
        int[] TablicaLiczb = new int[10];
        
        Kupon kupon = new Kupon(TablicaLiczb);
        
        
        for (int i = 0; i < TablicaLiczb.length; i++){
            System.out.println(TablicaLiczb[i]);
        }
    }
}

//I teraz wszelkie dalsze operację przeprowadzam na TablicaLiczb[] ?

Ogólnie dzięki, bo osiągnąłem ten efekt co w poście wyżej, ale prościej.

0

Niestety nie o to chodziło... Po co nam kupon, skoro on ma tylko generować tablicę z uporządkowanymi liczbami?

Obiekty mają sobą zarządzać... jak najwięcej.

Coś takiego wymodziłem na szybko... https://repl.it/Dnzu/6

0 użytkowników online, w tym zalogowanych: 0, gości: 0