Du verwendest einen veralteten Browser. Es ist möglich, dass diese oder andere Websites nicht korrekt angezeigt werden. Du solltest ein Upgrade durchführen oder ein alternativer Browser verwenden.
Hallo!
ich möchte eine Klasse schreiben, die Mitarbeiter mit gleichen Arbeitszeiten erst sortiert (mit dem übergebenen Comparator) und dann filtert um am Ende eine Liste zurückzugeben, die nur noch Mitarbeiter enthält deren Arbeitszeiten sich nicht überschneiden. Das ist soweit mein Ansatz:
Java:
public class TimeSelector {
public static List <Employee> selectDiffTimes(Comparator<Employee> comp,
Collection<Employee> col){
List <Employee> list = new ArrayList <Employee>();
List <Employee> choosenEmployees = new ArrayList <Employee>();
list.addAll(col);
list.sort(comp);
if(list.size() == 1){
return list;
}
for(Employee a : list){
for(int b = 0; b < list.size(); b++){
int x= 0;
//außer sich selbst
if(b != list.indexOf(a) && x== 0){
if(!list.get(b).getEnd().isAfter(a.getStart()) ||
!list.get(b).getStart().isBefore(a.getEnd())){
if(listSorted.contains(a)){
x++;
continue;
} else choosenEmployees.add(a);
}else continue;
}
}
}
listSorted.sort(comp);
return choosenEmployees;
}
}
Leider funktioniert er nicht, wie er soll, da auch Mitarbeiter ausgegeben werden, die zur selben Zeit arbeiten. Ich bin mir aber ziemlich sicher, dass die Bedingungen in der if stimmen sollten. Vertue ich mich da? Oder liegt der Fehler an einer anderen Stelle?
Also der Code ist schwer lesbar. Variablen einfach nach dem Alphabet zu benennen ist extrem ungeschickt und dann ist das Konstrukt extrem kompliziert aufgebaut!
a) So viele Verschachtelungen sind generell unleserlich. Da hilft es sehr, die Logik in weitere Funktionen auszulagern. Dies sorgt für einen lesbaren Code und ermöglichst das Testen von kleineren Einheiten.
b) Die Variable x nutzt Du nur, um aus der for Schleife auszusteigen, oder? Dann wäre doch evtl. die Variable x ersatzlos zu streichen und das continue durch break zu ersetzen? (Oder habe ich einen Teil der Logik auf die Schnelle falsch erfasst?) Wobei dies ggf. durch das Refactoring bezüglich Punkt a) ehh entfällt, weil man dann die ganze Unterfunktion einfach durch ein return verlassen kann.
c) Wieso packst Du erst alles in list? Kannst Du nicht direkt die übergebene Collection col nutzen?
d) Mal nutzt Du choosenEmployees und mal listSorted. Was ist listSorted? Die Variable sehe ich im lokalen Kontext nicht.
e) Deine if Bedingung verstehe ich nicht ganz. Da würde ich auch die Bedingung noch einmal genauer prüfen. Aber wenn Du das in einer Funktion auslagerst und dann mit Unit-Tests alle Fälle, die zu unterscheiden sind, prüfst, dann hast Du ja eine super Kontrolle, ob die Logik wirklich korrekt ist.
f) Debugger sind generell sehr sinnvoll um genau zu sehen, was genau passiert um das dann zu kontrollieren.
Noch ein Tipp zur if-Bedingung, die mir auch seltsam vorkommt: Bei solchen Überschneidungsabfragen ist es häufig einfacher, nicht die Überschneidungsbedingung zu formulieren, sondern die Bedingung, bei der es keine Überschneidung gibt. Die muß man dann nur noch verneinen. Konkret: Du willst prüfen, ob sich Arbeitszeiten überschneiden. Also überlegst du dir die verschiedenen Bedingungen unter denen sie sich garantiert nicht überschneiden, verknüpfst diese mit && und negierst den gesamten Ausdruck mit !.
keine Überschneidungen: start1 < start2 && ende1 < ende2 || start1 > start2 && ende1 > ende2
Überschneidungen: (start1 > start2 || ende1 > ende2) && (start1 < start2 || ende1 < ende2)
ganz korrekt ist das nicht, weil ende oder start auch in der Arbeitszeit des anderen liegen können
Nur jetzt jeden mit jeden vergleichen, halte ich für falsch, da gibt's Schlaueres
Edit: bitte auch beachten, Negation von < ist nicht >
Da habe ich Quatsch geschrieben. Statt && muß man mit || verknüpfen. Es gibt also keine Überschneidung wenn ende1 < start2 || ende2 < start1ist. Eine Überschneidung liegt also bei !(ende1 < start2 || ende2 < start1) vor.
Da habe ich Quatsch geschrieben. Statt && muß man mit || verknüpfen. Es gibt also keine Überschneidung wenn ende1 < start2 || ende2 < start1ist. Eine Überschneidung liegt also bei !(ende1 < start2 || ende2 < start1) vor.
Genau, ende1 < start2 bedeutet: Die Arbeitszeit liegt davor von 1, ende2 < start1 bedeutet: Die Arbeitszeit liegt danach von 1,
das ist so wie hier im Forum, der eine schläft halt mal und der andere ist wach,
und 48 Stunden möchte niemand wach sein, deswegen schläft man ab und an/zu
