Best Practice Code Refaktorisierung

[Prafy]

Hallo allerseits,
ich habe letztens ein paar Videos zum Thema Refaktorisierung gesehen. Und da habe ich festgestellt, dass das mein Code teilweise dringend nötig hätte. Daraufhin habe ich mein aktuelles Projekt überarbeitet, aber an einer Stelle habe ich keine Ahnung, wie ich das refaktorisieren könnte.

private Day[] createWeek(Day day)
    {
        DayOfWeek dayName = DayOfWeek.MONDAY;
        week = new Day[7];
        switch(day.getDay())
        {
        case MONDAY:
            for(int i = 0; i < 7; i++)
                {
                    week[i] = new Day(dayName.plus(i), (byte) (day.getDayNumber() + i), day.getMonth(), day.getYear());
                }
                week[0].setToday(true);
            break;
        case TUESDAY:
            week[0] = new Day(dayName, (byte) (day.getDayNumber() - 1), day.getMonth(), day.getYear());
            for(int i = 0; i < 6; i++)
            {           
                week[i + 1] = new Day(dayName.plus(i + 1), (byte) (day.getDayNumber() + i), day.getMonth(), day.getYear());
            }
            week[1].setToday(true);
            break;
        case WEDNESDAY:
            week[0] = new Day(dayName.plus(0), (byte) (day.getDayNumber() - 2), day.getMonth(), day.getYear());
            week[1] = new Day(dayName.plus(1), (byte) (day.getDayNumber() - 1), day.getMonth(), day.getYear());
            for(int i = 0; i < 5; i++)
            {           
                week[i + 2] = new Day(dayName.plus(i + 2), (byte) (day.getDayNumber() + i), day.getMonth(), day.getYear());
            }
            week[2].setToday(true);
            break;
        case THURSDAY:
            for(int i = 0, j = 3; i < 3; i++, j--)
            {
                week[i] = new Day(dayName.plus(i), (byte) (day.getDayNumber() - j), day.getMonth(), day.getYear());
            }
            for(int i = 0; i < 4; i++)
            {           
                week[i + 3] = new Day(dayName.plus(i + 3), (byte) (day.getDayNumber() + i), day.getMonth(), day.getYear());
            }
            week[3].setToday(true);
            break;
        case FRIDAY:
            for(int i = 0, j = 4; i < 4; i++, j--)
            {
                week[i] = new Day(dayName.plus(i), (byte) (day.getDayNumber() - j), day.getMonth(), day.getYear());
            }
            for(int i = 0; i < 3; i++)
            {           
                week[i + 4] = new Day(dayName.plus(i + 4), (byte) (day.getDayNumber() + i), day.getMonth(), day.getYear());
            }
            week[4].setToday(true);
            break;
        case SATURDAY:
            for(int i = 0, j = 5; i < 5; i++, j--)
            {
                week[i] = new Day(dayName.plus(i), (byte) (day.getDayNumber() - j), day.getMonth(), day.getYear());
            }
            for(int i = 0; i < 2; i++)
            {           
                week[i + 5] = new Day(dayName.plus(i + 5), (byte) (day.getDayNumber() + i), day.getMonth(), day.getYear());
            }
            week[5].setToday(true);
            break;
        default:
            for(int i = 0, j = 6; i < 7; i++, j--)
            {
                week[i] = new Day(dayName.plus(i), (byte) (day.getDayNumber() - j), day.getMonth(), day.getYear());
            }
            week[6].setToday(true);
            break;
        }
        return week;
    }

Kurze Erklärung zum Code: Die Methode bekommt einen beliebigen Wochentag per Parameter und errechnet dann die Woche drum herum. Wenn also ein Mittwoch der 14. reinkommt, berechnet er also den Dienstag den 13. und Montag den 12. und die darauffolgenden Tage bis Sonntag, sodass das ganze grafisch dann so aussieht:

Hat jemand eine Idee, wie ich das schöner machen, also refaktorisieren kann? Denn besonders wenn ich jetzt auch noch die Erkennung ausbaue, dass vor dem 1. eines Monats nicht der 0. und -1. kommt oder nach dem 31. nicht der 32. folgt, wird das noch verwirrender.
Falls jemand Ideen hat, wäre ich sehr dankbar. 🙂
Gruß,
Prafy

P.S.: Oder gibt es vielleicht sogar grundlegend andere Vorschläge, weil ich es mir wieder viel zu kompliziert mache? Hinweis: Ja, ich habe extra eine eigene Klasse Day geschrieben, die ich deshalb hier in der Methode auch so gesondert behandeln muss.

 

[mrBrown]

Ganz simple:
Der erste Tag der Woche ist der aktuelle Tag minus den Wochentagssindex (Mi,14 -> 14-2 = 12).
Die 7 Tage dann durchiterieren, starten mit dem ersten Tag und Montag.

Behandeln von Tagen kleiner 1 und größer 28/29/30/31 behandelst du dann entweder im Day-Konstruktor, oder eine extra Methode, die negative Tage/Monate etc zulässt und das passend umrechnet (ich würd letzteres machen)

 

[Thallius]

Warum benutzt du nicht localdate?

 

[Prafy]

Okay, vielen Dank, die Methode sieht nun so aus und funktioniert prächtig. 😀

private Day[] createWeek(Day day)
    {      
        DayOfWeek dayName = DayOfWeek.MONDAY;
        week = new Day[7];
      
        for(int i = 0, j = -(day.getDay().getValue() - 1); i < 7; i++, j++)
        {
            week[i] = new Day(dayName.plus(i), (byte) (day.getDayNumber() + j), day.getMonth(), day.getYear());
        }
        week[day.getDay().getValue()].setToday(true);
        return week;
    }

Manchmal denkt man einfach viel zu umständlich, aber mit ein paar Denkanstößen wie solchen kommt man doch meist gut weiter. 😀

LocalDate? Ich habe ein bisschen rumgegooglet und eigentlich nur Foren gesehen, in denen alles mit dem deprecated Date gemacht wurde. Dafür müsste man sich in der Java Bibliothek wohl ein bisschen besser auskennen. 😀
Ich schaue mir die Klasse mal an. Aber danke für den Tipp @Thallius

 

[mrBrown]

Manchmal denkt man einfach viel zu umständlich, aber mit ein paar Denkanstößen wie solchen kommt man doch meist gut weiter. 😀

Selbst das ginge noch besser, j kann man sich zB sparen, und stattdessen i+day.getDayNumber()+(day.getDay().getValue()+1) rechnen (und den zweiten Teil der Rechnung kann man auch noch in einer Variablen ablegen)

LocalDate? Ich habe ein bisschen rumgegooglet und eigentlich nur Foren gesehen, in denen alles mit dem deprecated Date gemacht wurde. Dafür müsste man sich in der Java Bibliothek wohl ein bisschen besser auskennen. 😀

Date solltest du auch besser ignorieren, die neue Date-API ist aber ziemlich gut zu benutzen, mittlerweile sollte man da auch einiges zu finden 😉

 

[Prafy]

Ihr immer mit euren besseren Lösungen. 😀
So, habe jetzt mal alles so einfach gemacht, wie man es hätte machen können und zwar so, wie ihr das vorgeschlagen habt (Also alles auf LocalDate umgeschrieben und die Methode gekürzt). Aber wenn es kompliziert geht, kann man es ja mal versuchen, mache ich meist leider immer als erstes. 😀
Der finale Code der Methode sieht so aus:

private Day[] createWeek(Day day)
    {
        week = new Day[7];
   
        LocalDate date = day.getDate();
   
        for(int i = 0, j = date.getDayOfWeek().getValue() - 1; i < 7; i++, j--)
        {
            week[i] = new Day(date.minusDays(j));
        }
   
        week[date.getDayOfWeek().getValue() - 1].setToday(true);
        CalendarPanel.showYear(date.getYear());
        return week;
    }

@mrBrown Bei dem, was du zuletzt gesagt hattest bin ich nicht ganz durchgestiegen. Trotzdem finde ich, dass das jetzt auch eine schöne Lösung ist, die auf jeden Fall funktioniert. 😉 Ich mag das 'j' halt sehr gerne. 😀

 

[mrBrown]

So etwa (ist dein alter Code):

private Day[] createWeek(Day day) {    
        DayOfWeek monday = DayOfWeek.MONDAY;
        week = new Day[7];
     
        int firstDayOfWeek = day.getDayNumber() - (day.getDay().getValue() + 1);

        for(int i = 0; i < 7; i++)
        {
            week[i] = new Day(monday.plus(i),
                 (byte) (firstDayOfWeek + i), day.getMonth(), day.getYear());
        }
        week[day.getDay().getValue()].setToday(true);
        return week;
    }

CalendarPanel.showYear(date.getYear());

würde ich aus der Funkion rausnehmen, das führt sonst irgendwann zu Fehlern (und macht ua UnitTests schwierig bis unmöglich).
Außerdem siehts nach static aus, was auch eher schlecht wäre...

 

[Prafy]

Hm ... also mit UnitTests und so habe ich mich noch nie beschäftigt. Ich will immer mein Wissen ein bisschen erweitern (ich will halt einfach ein guter Programmierer werden ^^), komme aber nie über viel mehr als das, was ich eben selber zusammencode, hinaus. Und ein Informatikstudium mache ich ja wahrscheinlich leider auch nicht. 🙁
Aber das showYear() lässt oben rechts im Kalender noch das Jahr anzeigen, etwa so:

Und all diese Day-Objekte (extends JPanel) sind in einem anderen JPanel vom Typ CalendarPanel eingegliedert. Um nun das Jahr anzuzeigen muss das CalendarPanel eben das Jahr über die Day-JPanels anzeigen. Und da das eigentliche CalendarPanel-Objekt ja nur der JFrame hat, kann ich da entweder nur über das JFrame zugreifen oder direkt in CalendarPanel über static, was ich hier gemacht habe. Ich weiß, dass das irgendwie rein refaktorisierungstechnisch nicht optimal ist, aber da weiß ich auch nicht wirklich, wie ich das anders strukturieren soll, weil das Jahr einfach über das CalendarPanel gehen muss, oder?

Übrigens: Im dem Bild, welches ich beim eröffnen vom Thread oben reingepackt habe, war das Jahr nicht zu sehen, weil es aus irgendeinem Grund von der leicht gelblichen Einfärbung des Tages überdeckt wurde. Das ist auch noch ein Problem, welches ich habe. Das heißt, dass jedes mal, wenn Montag ist, das Jahr durch die Tagesmarkierung überdeckt wird. ^^