Pytanie o poprawność konstruktora i metodę

Rejestracja:ponad 5 lat
Ostatnio:5 miesięcy
Postów:33

0

Witam
Mam do was dwa pytanka:

Posiadam taki konstruktor

Kopiuj

 public Match(FootballClub teamA, FootballClub teamB, int teamAScore, int teamBScore, Date date) {
        this.teamA = teamA;
        this.teamB = teamB;
        this.teamAScore = teamAScore;
        this.TeamBScore = teamBScore;
        this.date = date;
        if(teamAScore>teamBScore){
            teamA.setWinsCount(teamA.getWinsCount()+1);
            teamA.setScoredGoalsCount(teamA.getScoredGoalsCount()+teamAScore);
            teamA.setRecivedGoalsCount(teamA.getRecivedGoalsCount()+teamBScore);
            teamB.setDefeatsCount(teamB.getDefeatsCount()+1);
            teamB.setScoredGoalsCount(teamB.getScoredGoalsCount()+teamBScore);
            teamA.setRecivedGoalsCount(teamA.getRecivedGoalsCount()+teamBScore);
            teamA.setPoints(teamA.getPoints()+3);
        }
        else if(teamAScore==teamBScore){
            teamA.setDrawsCount(teamA.getDrawsCount()+1);
            teamB.setDrawsCount(teamB.getDrawsCount()+1);
            teamA.setRecivedGoalsCount(teamA.getRecivedGoalsCount()+teamBScore);
            teamA.setRecivedGoalsCount(teamA.getRecivedGoalsCount()+teamBScore);
            teamA.setPoints(teamA.getPoints()+1);
            teamB.setPoints(teamB.getPoints()+1);
        }
        else if(teamBScore>teamAScore){
            teamB.setWinsCount(teamB.getWinsCount()+1);
            teamB.setScoredGoalsCount(teamB.getScoredGoalsCount()+teamBScore);
            teamB.setRecivedGoalsCount(teamB.getRecivedGoalsCount()+teamAScore);
            teamA.setDefeatsCount(teamA.getDefeatsCount()+1);
            teamA.setScoredGoalsCount(teamA.getScoredGoalsCount()+teamAScore);
            teamB.setRecivedGoalsCount(teamB.getRecivedGoalsCount()+teamAScore);
            teamB.setPoints(teamB.getPoints()+3);
        }
    }

Czy umieszczenie takiej logiki w nim jest zgodne z normami, konwencjami, czy powinna być gdzie indziej?

2 Mam taką metodę i wywołanie

Kopiuj

 private Date setDate(){
        System.out.println("Podaj datę: (dd-mm-rrrr)");
        String line = scanner.nextLine();
        Date date = null;
        try {
            date = new SimpleDateFormat("dd-MM-yyyy").parse(line);
        } catch (ParseException ex) {
            System.out.println("niepoprawny format daty");
        }
        return date;
    }

Kopiuj

  private void playMatch() {
        Date date;
        do {
            date = setDate();
        }
        while(date==null);
}

Czy jest to dobre rozwiązanie, jeśli chce pytać aż do uzyskania poprawnej odpowiedzi? Czy jest lepsza opcja?
Z góry dzięki

Java

Charles_Ray

2020-08-08 19:17

Charles_Ray2020-08-08 19:17

Rejestracja:około 17 lat
Ostatnio:dzień
Postów:1875

3

Nie. W konstruktorach nie piszemy logiki. Pętlę do-while ostatni raz widziałem 10 lat temu, więc tutaj się nie wypowiem :)

EDIT: czytanka https://www.yegor256.com/2015/05/07/ctors-must-be-code-free.html

edytowany 5x, ostatnio: Charles_Ray 2020-08-08 20:11

welik2 2020-08-08 19:26

czyli lepiej by było gdyby w metodzie, użył konstruktora, a pod nim dał tą logikę?

Charles_Ray 2020-08-08 19:27

Dopisz testy, to zobaczysz z czym będzie Ci bardziej po drodze :) ja bym dał inicjalizację w jakiejś fabryce. Konstruktor służy do zadeklarowania zależności lub parametrów

Mateusz Handzlik

2020-08-09 13:07

Mateusz Handzlik2020-08-09 13:07

Rejestracja:prawie 5 lat
Ostatnio:ponad 4 lata
Postów:4

0

Chłopaki nie przepisuje książek. Na YT jest wszystko.

kixe52 2020-08-09 15:21

?

KamilAdam

2020-08-10 08:09

KamilAdam2020-08-10 08:09

Rejestracja:ponad 6 lat
Ostatnio:28 dni
Lokalizacja:Silesia/Marki
Postów:5505

3

Ad. 1. Zgadzam się z @Charles_Ray że konstruktor powinien być bez logiki. Powinieneś mieć jakąś metodę statyczną/wytwórczą calculateMatch/createMatch i tam robić te wyliczenia, a w konstruktorze ustawiać tylko wyliczone wartości
Ad. 2. Metoda o sygnaturze private Date setDate() która nie ustawia daty tylko ją pobiera od użytkownika wygląda jak ponury żart. BTW używanie nulli, zwłaszcza jako wartości zwracane, jest złe

Mama called me disappointment, Papa called me fat
Każdego eksperta można zastąpić backendowcem który ma się douczyć po godzinach. Tak zostałem ekspertem AI, Neo4j i Nest.js . Przez mianowanie

welik2 2020-08-10 12:31

Bardzo mi pomogłeś, a jeśli chodzi o nazwę, pierwotnie ustawiała datę, zapomniałem zmienić. A jak zastąpić tego nulla?

99xmarcin

2020-08-10 08:35

~~99xmarcin~~2020-08-10 08:35

Rejestracja:około 5 lat
Ostatnio:5 miesięcy
Postów:2420

1

Tu raczej trzeba się zastanowić jak to jest za-modelowane. Ja bym miał takie klasy:

Match - reprezentuje mecz, może być jeszcze nie rozegrany lub już rozegrany
MatchResult - reprezentuje wynik meczu (można też mieć MatchStats dla dokładnych statystyk)
Team reprezentuje drużyne
TeamStats - reprezentuje statystyki drużyny
Player i PlayerStats podobnie

A potem bym miał:

Kopiuj

match.finish();

// W match finish można mieć już
teams().foreach(t => t.updateStats(this.getMatchStats()));

Można też miec bardziej globalny obiekt typu FootballSeasonSimulator który by pilnował uaktualnienia statystyk po zakończeniu meczu.

welik2 2020-08-10 12:33

spróbuję tak to przerobić

jarekr000000

2020-08-10 08:52

jarekr0000002020-08-10 08:52

Rejestracja:ponad 8 lat
Ostatnio:około 6 godzin
Lokalizacja:U krasnoludów - pod górą
Postów:4707

3

I jeszcze jedno - java.util.Date (i java.util.Calendar) to bryndza. Omijaj.
Uzywaj klas z java.time:
https://docs.oracle.com/javase/8/docs/api/java/time/LocalDate.html
java.time.format.DateTimeFormatter

welik2 2020-08-10 12:33

okej, dzięki

Belka

2020-08-11 19:44

Belka2020-08-11 19:44

Rejestracja:prawie 8 lat
Ostatnio:3 dni
Lokalizacja:PL
Postów:452

1

Konkstruktor powinien być jak najprostszy. Jeżeli Twój przyjmuje zbyt dużo argumentów na wejściu, staje się również mniej czytelny. Zainteresuj się wtedy wzorcem Builder (nie wariantem z Gang of Four).

Julian_

2020-08-11 22:13

Julian_2020-08-11 22:13

Rejestracja:około 8 lat
Ostatnio:ponad 4 lata
Postów:1703

2

Zrób sobie klasę FootballCup niemutowalną i wtedy kulturalnie tworzenie tych klas będziesz musiał zrobić przed Match.
Poza tym potrzebujesz więcej klas:
np. klasę ze statystykami drużyny

edytowany 1x, ostatnio: Julian_ 2020-08-11 22:14

welik2 2020-08-11 22:20

obecnie wszystko mam w klasie FootballClub, lepiej byłoby jakbym stworzył ClubStatistics? i potem te ClubStatistics jako atrybut klasy dać?

Julian_ 2020-08-11 22:25

ciężko powiedzieć, bo nie znam całości, jest duzo mozliwosci...

Julian_

2020-08-12 16:56

Julian_2020-08-12 16:56

Rejestracja:około 8 lat
Ostatnio:ponad 4 lata
Postów:1703

1

Ja bym takim modelem pojechał np.:

Kopiuj

public class TeamStatstics {

    public final Integer matchesPlayed;
    public final Integer winMatches;
    public final Integer lostMatches;
    public final Integer scoredGoals;
    public final Integer receivedGoals;
    public final Integer points;

    // buildera se zrób

    public TeamStatstics(Integer matchesPlayed, Integer winMatches, Integer lostMatches, Integer scoredGoals, Integer receivedGoals, Integer points) {
        this.matchesPlayed = matchesPlayed;
        this.winMatches = winMatches;
        this.lostMatches = lostMatches;
        this.scoredGoals = scoredGoals;
        this.receivedGoals = receivedGoals;
        this.points = points;
    }


}

Kopiuj

public class LeaugeStatistics {

    // jakies atrybuty

    private Map<Long, TeamStatstics> statistics = new HashMap<>();

    // jakis konstruktor

    public synchronized void addTeamStatistics(Long teamId, TeamStatstics teamStat) {
        statistics.put(teamId, teamStat);
    }

    public TeamStatstics getTeamStatistics(Long teamId) {
        return statistics.get(teamId);
    }

}