Czy dobrze używam dziedziczenia i czy kod jest czytelny?

0

Czy dobrze używam dziedziczenia w tym przykładzie? Staram się tworzyć programy, żeby poćwiczyć, to, czego nauczyłem się z kursu. Napisałem prosty program, w którym twórzmy postać do gry rpg (na tą chwile klasy poszczególnych ras postaci nie maja dużo metod i pól, zanim dodam więcej chce wiedzieć, czy zmierzam w dobrym kierunku). Moim pytaniem jest, co mógłbym poprawić w kodzie, aby był czytelniejszy i lepiej napisany przynajmniej na poziomie, na którym jestem. Tutaj jest kod

import java.util.Scanner;

public class Main {
    public static void main(String[] args) throws Exception {
        Character[] characters = {
                new Human("Człowiek"),
                new Elf("Elf"),
                new Dwarf("Krasonlud"),
                new DarkElf("Elf Mroku"),

        };
        for (Character character: characters) {
            System.out.println("\n\n");
            character.setRace(character);
            System.out.println(STR."Tworzę postać \{character.getRace()}");
            character.setWeapon(character);
            System.out.println(character);
            character.attack();
        }
    }
}

abstract class Character {
    Scanner input = new Scanner(System.in);
    protected String name;
    protected String race;
    protected String[] races = {
            "Człowiek",
            "Elf",
            "Krasnolud",
            "Elf mroku",
            };
    protected String weapon;
    protected String[] weapons = {
            "Miecz",
            "Łuk",
            "Kij",
            "Sztylet",
            "Młot"};
    protected int hp = 100;
    protected void attack() {
        System.out.println(STR."Atakuje bronią \{weapon}");
    }
    protected void setRace(Character c) throws Exception {
        if (c instanceof Human) {
            c.race = races[0];
        } else if (c instanceof Elf) {
            c.race = races[1];
        } else if (c instanceof Dwarf) {
            c.race = races[2];
        } else if (c instanceof DarkElf) {
            c.race = races[3];
        } else {
            throw new Exception("Błąd w odnalezieniu rasy");
        }
    }
    public String getRace() {
        return race;
    }

    protected void setWeapon(Character c) {
        System.out.println("Jaka broń chcesz wybrać?");
        for (int i = 0; i < weapons.length; i++) {
            System.out.println(STR."\{i + 1}. \{weapons[i]}");
        }
        System.out.print("Twój wybór: ");
        int choice = input.nextInt();
        c.weapon = weapons[choice - 1];
    }

    @Override
    public String toString() {
        return STR."Mam na imię \{name}\nMam \{hp} punktów zdrowia\nMoja rasa to \{race}";
    }
}

class Human extends Character {

    Human(String name) {
        this.name = name;
    }

    @Override
    protected void attack() {
        System.out.println("Atak człowieka");
        super.attack();
    }
}

class Elf extends Character {
    Elf(String name) {
        this.name = name;
    }
    @Override
    protected void attack() {
        System.out.println("Atak Elfa");
        super.attack();
    }

}

class Dwarf extends Character {
    Dwarf(String name) {
        this.name = name;
    }
    @Override
    protected void attack() {
        System.out.println("Atak Krasnoluda");
        super.attack();
    }

}

class DarkElf extends Character {
    DarkElf(String name) {
        this.name = name;
    }
    @Override
    protected void attack() {
        System.out.println("Atak Elfa Mroku");
        super.attack();
    }

}

4
  1. Zdecydowanie odradzam używania jakichkolwiek nazw kluczowych jako nazw klas/zmiennych (u Ciebie "Character")
  2. races powinno być enumem a nie Stringiem

Ale ogólnie błędów jest tak dużo, że ciężko powiedzieć cokolwiek, po prostu nie tak się pisze w javie. Nie piszę tego, żeby Cię zniechęcić tylko żebyś wiedział, bo coś tam umiesz tylko jeszcze sporo drogi przed Tobą. Radziłbym Ci wziąć do ręki jakąś książkę, najlepiej "Thinking in Java" albo "Java 8 Lambdas" - tam są fajne przykłady jak powinna wyglądać hierarchia klas.

import java.util.Arrays;

public class FantasyWorld {
    public static void main(String[] args) throws Exception {
        FantasyCharacter[] characters = {
                new Human("Człowiek"),
                new Elf("Elf"),
                new Dwarf("Krasonlud"),
        };
        Arrays.stream(characters).forEach(FantasyCharacter::attack);
    }
}

enum Race {
    HUMAN, ELF, DRAWF, DARKELF
}

abstract class FantasyCharacter {
    protected String name;
    protected Race race;
    protected String weapon;
    protected int hp = 100;
    protected void attack() {
        System.out.println("Atakuje bronią \{weapon}");
    }
    public Race getRace() {
        return race;
    }
}

class Human extends FantasyCharacter {
    public Human(String name) {
        this.name = name;
        this.race = HUMAN;
    }
    @Override
    protected void attack() {
        System.out.println("Atak człowieka");
        super.attack();
    }
}

class Elf extends FantasyCharacter {
    public Elf(String name) {
        this.name = name;
        this.race = ELF;
    }
    @Override
    protected void attack() {
        System.out.println("Atak Elfa");
        super.attack();
    }
}

class Dwarf extends FantasyCharacter {
    public Dwarf(String name) {
        this.name = name;
        this.race = DRAWF;
    }
    @Override
    protected void attack() {
        System.out.println("Atak Krasnoluda");
        super.attack();
    }
}

5

Na wstępie powiem że źle, i to na dwóch płaszczyznach.

  1. Po pierwsze, Twoje klasy źle po sobie dziedziczą, chociażby dlatego że Character wie o pod klasach. Poza tym te instanceof wszędzie, szkoda gadać. Nie tak się powinno pisać. Cały ten kod to crap, niestety :/
  2. Po drugie dlatego, że programowanie obiektowe to nie jest coś co można w sensowny sposób wytłumaczyć analogią do postaci - tak jak w Twoim przykładzie. Do niczego dobrego to nie zaprowadzi. Ogólnie wszystkie kursy które próbują uczyć klas na podstawie takich rzeczy jak: postaci, planety, samochody, owoce, to wszystko jest crap. Nie polecam, 0/10.
phantom_wizard napisał(a):
  1. Zdecydowanie odradzam używania jakichkolwiek nazw kluczowych jako nazw klas/zmiennych (u Ciebie "Character")

Nie zgadzam się. Po to są przestrzenie nazw, żeby sobie nazywać klasy jak kto chce.

4

Dziedziczenia używasz jak masz inną logikę pod tym samym interfejsem. Dalej: jak logika jest inna, ale jedyny punkt zmienny to są jakieś dane to np. nazwa postaci to robisz jedną klasę. Osobne klasy/dziedziczenie robisz tylko wtedy jak kod wewnątrz metod się różni.

W tym wypadku nie ma to kompletnie sensu. Powinieneś mieć jedną klasę Character i tyle. Projektując kod OOP nie myśl o tym czym coś jest fizycznie (duży błąd wszędzie powielany) tylko to co coś dane robi.

Jak chcesz poczytać jak prawilnie w grach implementuje się taką złożoną logię to poczytaj o https://en.wikipedia.org/wiki/Entity_component_system

1
phantom_wizard napisał(a):
  1. Zdecydowanie odradzam używania jakichkolwiek nazw kluczowych jako nazw klas/zmiennych (u Ciebie "Character")

Nie zgadzam się. Po to są przestrzenie nazw, żeby sobie nazywać klasy jak kto chce.

Baaardzo śliska argumentacja. To, że coś jest wykonalne nie znaczy że powinno się tak robić. Jako przykład podam notkę z polskiej blogosfery dot. javy której teraz nie znajdę, ale chodziło o zaimportowanie błędnej adnotacji ze złej przestrzeni nazw. Na pewno dla kogoś kto panuje nad projektem jest to łatwiejsze do wychwycenia, ale nie zawsze pracuje się z kodem który się napisało.

2
phantom_wizard napisał(a):

Nie zgadzam się. Po to są przestrzenie nazw, żeby sobie nazywać klasy jak kto chce.

Baaardzo śliska argumentacja. To, że coś jest wykonalne nie znaczy że powinno się tak robić.

True, ale jeśli wziąć Twój argument - tzn. że nie powinienem nazywać klas nazwą która już istnieje, czyli równie dobrze mógłbym w ogóle nie używać przestrzeni nazw.

phantom_wizard napisał(a):

Jako przykład podam notkę z polskiej blogosfery dot. javy której teraz nie znajdę, ale chodziło o zaimportowanie błędnej adnotacji ze złej przestrzeni nazw. Na pewno dla kogoś kto panuje nad projektem jest to łatwiejsze do wychwycenia, ale nie zawsze pracuje się z kodem który się napisało.

Nawet jeśli tak jest, to odpowiedzią na to na pewno nie jest "po prostu nie nazywaj klas tak samo", tylko albo napisać odpowiednie testy po to, albo ogarnąć to odpowiednimi typami, albo po prostu nauczyć się normalnie pracować z przestrzenami nazw.

slsy napisał(a):

W tym wypadku nie ma to kompletnie sensu. Powinieneś mieć jedną klasę Character i tyle. Projektując kod OOP nie myśl o tym czym coś jest fizycznie (duży błąd wszędzie powielany) tylko to co coś dane robi.

Jak chcesz poczytać jak prawilnie w grach implementuje się taką złożoną logię to poczytaj o https://en.wikipedia.org/wiki/Entity_component_system

Nie wierzę że to mówię, ale zgadzam się. +1.

slsy napisał(a):

Dziedziczenia używasz jak masz inną logikę pod tym samym interfejsem. Dalej: jak logika jest inna, ale jedyny punkt zmienny to są jakieś dane to np. nazwa postaci to robisz jedną klasę. Osobne klasy/dziedziczenie robisz tylko wtedy jak kod wewnątrz metod się różni.

To chyba określenie polimorfizmu, a nie dziedziczenia? Tak, wiem że w Javie to jest bardzo zbliżone, ale w innych językach nie, więc może lepiej nie mieszać autorowi.

0

Dzięki, zapamiętam to

phantom_wizard napisał(a):

Zdecydowanie odradzam używania jakichkolwiek nazw kluczowych jako nazw klas/zmiennych (u Ciebie "Character")

Jeśli chodzi o to, to nie mialem jeszcze tego w kursie, ale mniej więcej wiem o co chodzi, więc zaraz poprawie

races powinno być enumem a nie Stringiem

Problem jest taki, że ucząc się tego z kursu jest wytłumaczona składnia np klas, ogólnie kiedy się stosuje i przykład np na podstawie takiej postaci i nie wiem skąd mogę się nauczyć pisać poprawnie w Javie, a nie tak, jak teraz

Ale ogólnie błędów jest tak dużo, że ciężko powiedzieć cokolwiek, po prostu nie tak się pisze w javie. Nie piszę tego, żeby Cię zniechęcić tylko żebyś wiedział, bo coś tam umiesz tylko jeszcze sporo drogi przed Tobą.

Spojrzałem przed chwilą na te książki i widzę, ze najnowsze wydanie tej pierwszej jest chyba z 2006 roku, więc mam pytanie, czy wciąż warto ją kupić pomimo tego, że ma prawie 20 lat, czy takie rzeczy raczej nie uległy większym zmianom. Ta druga widzę też jest chyba z 2014 roku

Radziłbym Ci wziąć do ręki jakąś książkę, najlepiej "Thinking in Java" albo "Java 8 Lambdas" - tam są fajne przykłady jak powinna wyglądać hierarchia klas.

O którą część kodu dokładnie chodzi?

Riddle napisał(a):

Na wstępie powiem że źle, i to na dwóch płaszczyznach.

  1. Po pierwsze, Twoje klasy źle po sobie dziedziczą, chociażby dlatego że Character wie o pod klasach.

Dlaczego nie powinno się używać instanceof?

Poza tym te instanceof wszędzie, szkoda gadać. Nie tak się powinno pisać. Cały ten kod to crap, niestety :/

Czyli na jakich przykładach powinno się to tłumaczyć? Może wtedy lepiej zrozumiem

Po drugie dlatego, że programowanie obiektowe to nie jest coś co można w sensowny sposób wytłumaczyć analogią do postaci - tak jak w Twoim przykładzie. Do niczego dobrego to nie zaprowadzi. Ogólnie wszystkie kursy które próbują uczyć klas na podstawie takich rzeczy jak: postaci, planety, samochody, owoce, to wszystko jest crap. Nie polecam, 0/10.

1
Maciek_SK8 napisał(a):

O którą część kodu dokładnie chodzi?

Riddle napisał(a):

Na wstępie powiem że źle, i to na dwóch płaszczyznach.

  1. Po pierwsze, Twoje klasy źle po sobie dziedziczą, chociażby dlatego że Character wie o pod klasach.

Dlaczego nie powinno się używać instanceof?

W ogólności dlatego: https://pl.wikipedia.org/wiki/Zasada_podstawienia_Liskov Klasy dziedziczące nie powinny wiedzieć nic o innych klasach (poza swoimi bazowymi) aby nie zmieniać kodu gdybyś chciał którąkolwiek usunąć. W java jest wiele mechanizmów których można użyć, ale się nie powinno i jednym z nich jest w takim przypadku instanceof.

0

Czyli z tego, co rozumiem i przykłady, które robię nie maja na ta chwile żadnego sensu na tym etapie nauki, jeśli chodzi o dziedziczenie?

slsy napisał(a):

Dziedziczenia używasz jak masz inną logikę pod tym samym interfejsem. Dalej: jak logika jest inna, ale jedyny punkt zmienny to są jakieś dane to np. nazwa postaci to robisz jedną klasę. Osobne klasy/dziedziczenie robisz tylko wtedy jak kod wewnątrz metod się różni.

Dziedziczenie miałoby sens wtedy, kiedy np. każda rasa miałaby np. jakieś swoje unikalne metody?

W tym wypadku nie ma to kompletnie sensu. Powinieneś mieć jedną klasę Character i tyle.

Średnio rozumiem o co chodzi

Projektując kod OOP nie myśl o tym czym coś jest fizycznie (duży błąd wszędzie powielany) tylko to co coś dane robi.

Dzięki zobaczę, ale nie chodzi mi konkretnie o tworzenie gier, tylko o praktyczną naukę tego, czego się nauczyłem podczas kursu

Jak chcesz poczytać jak prawilnie w grach implementuje się taką złożoną logię to poczytaj o https://en.wikipedia.org/wiki/Entity_component_system

Dzięki, na pewno będę o tym pamiętał następnym razem

W ogólności dlatego: https://pl.wikipedia.org/wiki/Zasada_podstawienia_Liskov Klasy dziedziczące nie powinny wiedzieć nic o innych klasach (poza swoimi bazowymi) aby nie zmieniać kodu gdybyś chciał którąkolwiek usunąć

Dobra, to nie będę już tego używał

. W java jest wiele mechanizmów których można użyć, ale się nie powinno i jednym z nich jest w takim przypadku instanceof.

3
Maciek_SK8 napisał(a):

Czyli z tego, co rozumiem i przykłady, które robię nie maja na ta chwile żadnego sensu na tym etapie nauki, jeśli chodzi o dziedziczenie?

No w sumie tak.

Maciek_SK8 napisał(a):

Dziedziczenie miałoby sens wtedy, kiedy np. każda rasa miałaby np. jakieś swoje unikalne metody?

W tym wypadku nie ma to kompletnie sensu. Powinieneś mieć jedną klasę Character i tyle.

Na ten moment całkowicie się tym nie przejmuj. Po prostu nie stosuj tego na razie, bo to jest jakby krok dalej - ma to swoje zastosowania, ale nie daje prawie niczego czego nie można ogarnąć ifami, pętlami i funkcjami (są oczywiście rzeczy które są ułatwione, ale one zaczynają być sensowne na "wyższym poziomie").

Maciek_SK8 napisał(a):

Średnio rozumiem o co chodzi

Projektując kod OOP nie myśl o tym czym coś jest fizycznie (duży błąd wszędzie powielany) tylko to co coś dane robi.

Po prostu olej rady kursu o klasach i obiektach.

0

Rozumiem

Na ten moment całkowicie się tym nie przejmuj. Po prostu nie stosuj tego na razie, bo to jest jakby krok dalej - ma to swoje zastosowania, ale nie daje prawie niczego czego nie można ogarnąć ifami, pętlami i funkcjami (są oczywiście rzeczy które są ułatwione, ale one zaczynają być sensowne na "wyższym poziomie").

Czyli mam po prostu iść dalej z materiałem i później w miarę się wszystko objaśni?

Po prostu olej rady kursu o klasach i obiektach.

2
Maciek_SK8 napisał(a):

Czyli mam po prostu iść dalej z materiałem i później w miarę się wszystko objaśni?

Po prostu olej rady kursu o klasach i obiektach.

No "samo" raczej się nie objaśni. Żeby to miało dla Ciebie jakikolwiek sens to musisz umieć pisać chociażby średnio-ogarnięte programy używając ifów, pętli i funkcji, które są "w miare okej", czyli: DRY, SRP, KISS, YAGNI.

Potem można się pokwapić żeby dodać klasę gdzieś (ale bez dziedziczenia, bez metod statycznych i bez polimorfizmu), i znowu pisać jakiś czas tak programy - co jakiś czas zbierając feedback od kogoś kto się zna.

0

Co to znaczy "średnio ogarnięte"? Mam na myśli, co powinien taki kod zawierać. Mógłbyś podać jakiś przykład ćwiczenia?

No "samo" raczej się nie objaśni. Żeby to miało dla Ciebie jakikolwiek sens to musisz umieć pisać chociażby średnio-ogarnięte programy używając ifów, pętli i funkcji, które są "w miare okej", czyli: DRY, SRP, KISS, YAGNI.

Czyli, jak już będę pisał te programy, to od czasu do czasu cos tutaj wrzucić do oceny błędów itd?

Potem można się pokwapić żeby dodać klasę gdzieś (ale bez dziedziczenia, bez metod statycznych i bez polimorfizmu), i znowu pisać jakiś czas tak programy - co jakiś czas zbierając feedback od kogoś kto się zna.

2
Maciek_SK8 napisał(a):

Co to znaczy "średnio ogarnięte"? Mam na myśli, co powinien taki kod zawierać. Mógłbyś podać jakiś przykład ćwiczenia?

No "samo" raczej się nie objaśni. Żeby to miało dla Ciebie jakikolwiek sens to musisz umieć pisać chociażby średnio-ogarnięte programy używając ifów, pętli i funkcji, które są "w miare okej", czyli: DRY, SRP, KISS, YAGNI.

No że będziesz się czuł pewnie napisać większość programów, bo właściwie każdy program da się napisać ze zmiennymi, pętlami, ifami i funkcjami.

1

Po pierwsze musisz nauczyć się totalnych podstaw i to nie byle jak tylko wkuć w pamięć podstawowe konstrukcje, klasy, interfejsy. Potem znajdź albo wymyśl w jaki sposób chcesz użyć nowo zdobytą wiedzę, na początku porób ćwiczenia, potem wymyślaj własne, rozkminiaj problemy przy zasypianiu. W internecie znajdziesz mnóstwo przykładowych problemów, najlepiej użyj wzorców projektowych, przykłady z brzegu:

  1. napisz klasę która będzie przesuwać napisy w pliku .srt
  2. zamodeluj klasy realizujące grę w szachy/warcaby/go - jakiego wzorca projektowego użyjesz?
1
phantom_wizard napisał(a):

Po pierwsze musisz nauczyć się totalnych podstaw i to nie byle jak tylko wkuć w pamięć podstawowe konstrukcje, klasy, interfejsy. Potem znajdź albo wymyśl w jaki sposób chcesz użyć nowo zdobytą wiedzę, na początku porób ćwiczenia, potem wymyślaj własne, rozkminiaj problemy przy zasypianiu. W internecie znajdziesz mnóstwo przykładowych problemów, najlepiej zacznij trenuj do tego wzorce projektowe rozwiązując problemy, przykłady z brzegu:

  1. napisz klasę która będzie przesuwać napisy w pliku .srt

To jest bardzo spoko.

phantom_wizard napisał(a):
  1. zamodeluj klasy realizujące grę w szachy/warcaby/go - jakiego wzorca projektowego użyjesz?

Tego nie polecam. W OO nie chodzi o modelowanie niczego, to jest złudny kierunek.

4
Riddle napisał(a):
  1. Po drugie dlatego, że programowanie obiektowe to nie jest coś co można w sensowny sposób wytłumaczyć analogią do postaci - tak jak w Twoim przykładzie. Do niczego dobrego to nie zaprowadzi. Ogólnie wszystkie kursy które próbują uczyć klas na podstawie takich rzeczy jak: postaci, planety, samochody, owoce, to wszystko jest crap. Nie polecam, 0/10.

Problem jest taki, że książki tłumaczą dziedziczenie jako taksonomię, najczęściej w postaci drzewka (mamy postać, człowiek jest to rodzaj postaci, wojownik to rodzaj człowieka itp.), podczas gdy w rzeczywistości dziedziczenie jest po prostu jedną z metod (nie jedyną) na współdzielenie implementacji. Nie ma tam jakiejś wielkiej mistyki, tylko po prostu masz kod ogólny i chcesz, żeby przypadki szczegółowe miały dostęp do tego kodu/implementacji. I rzadko zachodzi potrzeba robienia więcej niż 1 poziom dziedziczenia, jeżeli w ogóle jakieś stosować, bo można się obyć bez niego (stosując kompozycję albo rzeczy podobne do dziedziczenia, ale trochę się różniące od niego np. implementacja interfejsów w OOP pomaga osiągnąć polimorfizm bez dziedziczenia implementacji).

Jak na ironię w kodzie z powyższego posta to współdzielenie implementacji nie nastąpiło i każda klasa ustawia sobie name w konstruktorze:

this.name = name;

Ironią jest też, że w sytuacji, kiedy trzeba zrobić taksonomię właśnie i odróżniać jeden rodzaj czegoś od drugiego rodzaju czegoś (np. człowieka od elfa), to zwykle bardziej wygodnie zrobić to w inny sposób. Obiekty mogłyby mieć jakąś zmienną, która oznaczałaby typ postaci - ta zmienna w zależności od implementacji mogłaby być np. enumem, stringiem, liczbą(oznaczającą id typu), czy nawet referencją do jakiegoś innego obiektu, który zawierałby typowe rzeczy dla danego typu.

Natomiast często nie ma potrzeby odróżniać typów. Jeżeli jedna postać od drugiej różni się tylko tekstem, który mówi, to wystarczy zrobić pole string oznaczające zcustomizowany dla danego obiektu tekst, a nie całą klasę.

0
LukeJL napisał(a):

Natomiast często nie ma potrzeby odróżniać typów. Jeżeli jedna postać od drugiej różni się tylko tekstem, który mówi, to wystarczy zrobić pole string oznaczające zcustomizowany dla danego obiektu tekst, a nie całą klasę.

Prawda, natomiast wówczas całe powyższe ćwiczenie traci sens ;) Lepszym przykładem imo byłaby jakaś logika w odniesieniu do konkretnego typu pliku, ale znowu wciąganie dodatkowej logiki komplikuje przykład który jest tylko narzędziem w tłumaczeniu konceptu.

0
phantom_wizard napisał(a):
LukeJL napisał(a):

Natomiast często nie ma potrzeby odróżniać typów. Jeżeli jedna postać od drugiej różni się tylko tekstem, który mówi, to wystarczy zrobić pole string oznaczające zcustomizowany dla danego obiektu tekst, a nie całą klasę.

Prawda, natomiast wówczas całe powyższe ćwiczenie traci sens ;)

Nie "wtedy". To całe ćwiczenie nie ma sensu od początku.

phantom_wizard napisał(a):

Lepszym przykładem imo byłaby jakaś logika w odniesieniu do konkretnego typu pliku, ale znowu wciąganie dodatkowej logiki komplikuje przykład który jest tylko narzędziem w tłumaczeniu konceptu.

Ale to jest złe tłumaczenie. Nie w taki sposób, jeśli już się korzysta z dziedziczenia i polimorfizmu.

0

@Riddle

W OO nie chodzi o modelowanie niczego, to jest złudny kierunek.

Skądże. OO to narzędzie do modelowania.

1

Jak już robisz filmiki, polecam ten kanał, tych filmów nie widziałem ale inne wytłumaczyły mi pewne koncepty na początku zupełnie inaczej i lepiej niż te durne kursy.

1
Kerubyte napisał(a):

Jak już robisz filmiki, polecam ten kanał, tych filmów nie widziałem ale inne wytłumaczyły mi pewne koncepty na początku zupełnie inaczej i lepiej niż te durne kursy.

Jezus.

Dajcie spokój z przykładami na zwierzętach. Takie przykłady Dog extends Animal nic nie daje. Nie w taki sposób się używa polimorfizmu.

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