Zliczanie samogłosek w zdaniu //DO OCENY

0

Witam wszystkich,
Zacząłem uczyć się javy i ogólnie programowania, ale niestety nie mam nikogo kto mógłby spojrzeć na mój kod z góry i 'skrytykować'.
Wszystkie cenne uwagi bardzo mi się przydadzą.
Może pomożecie ? :)

class Licz{
String slowo; 
int licznik=0;

    Licz(String slowo){
        this.slowo=slowo;
        licz();
        System.out.print(wypisz());
    }
    
    int licz(){
        for(int i=0;i<slowo.length();i++){
            if(slowo.charAt(i)=='a'||slowo.charAt(i)=='e'||slowo.charAt(i)=='i'||slowo.charAt(i)=='o'||slowo.charAt(i)=='u'||slowo.charAt(i)=='y')
            licznik++;
        }
        return licznik;
    }
    
    String wypisz(){
        return licznik+" ";
    }

    
    public static void main(String [] args){
        Licz l1=new Licz("alicja");
        Licz l2=new Licz("kadafi ma kotka");
    }

}
1
  1. Po co robić z tego na siłę całą klasę? Jak już to lepiej jakąś StringHelper z metodą statyczną countVowels(String str).
  2. Dlaczego metoda wypisz coś zwraca, a nie wypisuje? I to dodatkowo zwraca ze spacją! Oczywiście metoda wypisz jest zbędna. Nie powinno być metody wypisującej cokolwiek, tym zajmuje się użytkownik poza klasą. Potrzebna jest za to metoda zwrocLiczbeSamoglosek... albo coś o lepszej nazwie, ale takim sposobie działania.
  3. Konstruktor nie powinien nic wypisywać.
  4. A wywołaj dwa razy licz ;)
    Ja napisałbym to tak:
class StringHelper {
 
	public static boolean isVowel(char ch) {
		return (ch == 'a') || (ch == 'e') || ...; // plus minus, pewnie w dżawie da się to zrobić ładniej, ale że nie piszę na co dzień w tym języku, to nie wiem
	}
	
	public static int countVowels(String str) {
		int vowels = 0;
		
		for (char ch: str.toCharArray()) {
			if (StringHelper.isVowel(ch)) {
				++vowels;
			}
		}
		
		return vowels;
	}
}

A potem wywołanie to np. System.out.println(StringHelper.countVowels("hello world"));. Prawda, że od razu bardziej przejrzyście?

1

Ten kod nie zlicza samogłosek, sprawdź ile znajdzie samogłosek w słowach "Alicja" i "żółw". Jeżeli nie chcesz uwzględniać polskich samogłosek, to wykorzystaj chociaż metodę toLowerCase (toUpperCase).
Nazwa metody wypisz jest zupełnie nieadekwatna do tego co ta metoda robi.
Konstruktor powinien konstruować obiekt, a nie wykonywać obliczenia, w konstruktorze nie powinno być metod licz, wypisz i println.
Moim zdaniem, konstruktor jest w tym programie zbędny.

public class Vowels
{
    private static String vowels = "aąeęioóuy";
    public static void main(String[] args)
    {
        System.out.println(countVowels("Alicja w krainie czarów"));
    }
    private static int countVowels(String txt)
    {
        int counter = 0;
        txt = txt.toLowerCase();
        for(char c: txt.toCharArray())
        {
            if(vowels.indexOf(c) >= 0)
            {
                counter++;
            }
        }
        return counter;
    }
}
0

Chyba powinienem jeszcze bardziej się cofnąć jeśli prawie nic nie rozumiem z waszych kodów :D Ale parę godzin walki i zrozumiałem (jeśli się mylę to poprawcie).
Jeden zmienia słowo na tablice znaków i przekazuje znak do funkcji isVowel, która sprawdza czy znak jest równy znakowi w funkcji (w tym wypadku z samogłoskami).
Drugi także zmienia słowo na tablicę znaków i przekazuje znak do funkcji wbudowanej indexOf, która sprawdza ilość wystąpień znaku w zmiennej vowels (w tym wypadku z samogłoskami).

Patrząc na wasze kody, to lepiej rozkładać zadanie na mniejsze pod zadania. Funkcja/zmienna która zawiera samogłoski i funkcja która porównuje znaki. Ale jakim błędem jest moje rozwiązanie ? Oprócz nazwy funkcji i błędów stylistycznych ? Operuję chyba na jednej zmiennej którą przechowuję w pamięci zamiast deklarować tablicę. Chyba, że chodzi tutaj o ciągłe szukanie litery o danym indeksie i przez to procesor się męczy.. ??

int licz(){
        for(int i=0;i<slowo.length();i++){
            if(slowo.charAt(i)=='a'||slowo.charAt(i)=='e'||slowo.charAt(i)=='i'||slowo.charAt(i)=='o'||slowo.charAt(i)=='u'||slowo.charAt(i)=='y')
            licznik++;
        }
        return licznik;
    }
1

indexOf, która sprawdza ilość wystąpień znaku w zmiennej vowels

indexOf nie sprawdza liczby wystąpień, tylko zwraca pierwszy indeks, gdzie dany znak/podciąg występuje (bądź -1, jeśli nie występuje).
http://www.tutorialspoint.com/java/java_string_indexof.htm

Patrząc na wasze kody, to lepiej rozkładać zadanie na mniejsze pod zadania.

Na tym właśnie opiera się programowanie ;)

Ale jakim błędem jest moje rozwiązanie ?

Nie rób spacji przed znakiem zapytania;
Jeśli mówisz o swoim pierwszym kodzie, to błędy masz już wytknięte.

Operuję chyba na jednej zmiennej którą przechowuję w pamięci zamiast deklarować tablicę.

?

Chyba, że chodzi tutaj o ciągłe szukanie litery o danym indeksie i przez to procesor się męczy.. ??

Nie zamęczysz żadnego procesora szukaniem samogłosek w wyrazie. Poza tym nie trudnij się premature optimization - kod ma przede wszystkim być pisany dla ludzi do czytania, a dopiero na samym końcu do kompilowania przez komputer.

2

Patrząc na wasze kody, to lepiej rozkładać zadanie na mniejsze pod zadania.
Tak jest taka taktyka, że jak masz jakiś duży skomplikowany problem to może być Ci łatwiej rozłożyć to na kilka mniejszych problemów. To tak jakbyś miał przenieść 200kg kamieni i zamiast zastanawiać się jak temu dać radę możesz to podzielić na 10x 20kg i będzie Ci łatwiej.

Ale jakim błędem jest moje rozwiązanie ?
no za duży ten kod. zrób to w jednej linijce używając StringUtils od Apache Commons albo np. "alicja".toLowerCase().replaceAll("[^aeiou]", "").length()
to jest za prosty problem żebyś tutaj coś dzielił, wydzielał, wymyślał i wydziwiał itd.

PS oceniam na 3 na szynach

2

Pisząc funkcję tak:

int licz(){
    for(int i=0;i<slowo.length();i++){
        if(slowo.charAt(i)=='a'||slowo.charAt(i)=='e'||slowo.charAt(i)=='i'||slowo.charAt(i)=='o'||slowo.charAt(i)=='u'||slowo.charAt(i)=='y') //***
        licznik++;
    }
    return licznik;
}

oszczędzasz minimalnie pamięć, pogarszasz wydajność (wielokrotne wywoływanie funkcji charAt z takim samym argumentem) i znacznie pogarszasz czytelność. Zauważ, że wiersz *** powinien mieć jeszcze 12 członów.

0

Okej, ukazaliście wszystko czego brakuje w moim kodzie ;)
Lekcja od was do przyjęcia, to teraz do zobaczenia przy następnym kodzie :)

Zobaczymy ile wyniosłem z tego wszystkiego :D

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