Je mehr ein Entwickler in einem Team an einer Anwendung arbeitet und je besser er seinen Code kennt, desto häufiger beschäftigt er sich mit dem Korrekturlesen der Arbeit seiner Kameraden. Heute werde ich zeigen, was in einer Woche in Code gefangen werden kann, der von sehr guten Entwicklern geschrieben wurde. Unter dem Schnitt befindet sich eine Sammlung lebendiger Artefakte unserer Kreativität (und ein wenig meiner Gedanken).
Komparatoren
Es gibt einen Code:
@Getter class Dto { private Long id; }
Jemand bemerkt, dass Sie den Stream direkt sortieren können, aber ich möchte Ihre Aufmerksamkeit auf den Komparator lenken. In der Dokumentation zur Comparator::compare Methode heißt es in Englisch in Weiß:
Vergleicht die beiden Argumente für die Reihenfolge. Gibt eine negative Ganzzahl, Null oder eine positive Ganzzahl zurück, da das erste Argument kleiner, gleich oder größer als das zweite ist.
Dieses Verhalten ist in unserem Code implementiert. Was ist los Tatsache ist, dass die Entwickler von Java sehr weitsichtig vorgeschlagen haben, dass viele einen solchen Komparator benötigen würden, und ihn für uns gemacht haben. Wir können es verwenden, indem wir unseren Code vereinfachen:
List<Long> readSortedIds(List<Dto> list) { List<Long> ids = list.stream().map(Dto::getId).collect(Collectors.toList()); ids.sort(Comparator.naturalOrder()); return ids; }
Ebenso dieser Code
List<Dto> sortDtosById(List<Dto> list) { list.sort(new Comparator<Dto>() { public int compare(Dto o1, Dto o2) { if (o1.getId() < o2.getId()) return -1; if (o1.getId() > o2.getId()) return 1; return 0; } }); return list; }
mit einem Handgriff verwandelt sich in
List<Dto> sortDtosById(List<Dto> list) { list.sort(Comparator.comparing(Dto::getId)); return list; }
Übrigens wird es in der neuen Version von „Ideas“ möglich sein, Folgendes zu tun:
Optionaler Missbrauch
Wahrscheinlich hat jeder von uns so etwas gesehen:
List<UserEntity> getUsersForGroup(Long groupId) { Optional<Long> optional = Optional.ofNullable(groupId); if (optional.isPresent()) { return userRepository.findUsersByGroup(optional.get()); } return Collections.emptyList(); }
Oft wird Optional verwendet, um das Vorhandensein / Fehlen eines Werts zu überprüfen, obwohl dieser nicht dafür erstellt wurde. Missbrauch binden und einfacher schreiben:
List<UserEntity> getUsersForGroup(Long groupId) { if (groupId == null) { return Collections.emptyList(); } return userRepository.findUsersByGroup(groupId); }
Denken Sie daran, dass es bei Optional nicht um das Methodenargument oder -feld geht, sondern um den Rückgabewert. Aus diesem Grund wurde es ohne Serialisierungsunterstützung entwickelt.
void Methoden, die den Status eines Arguments ändern
Stellen Sie sich eine Methode wie diese vor:
@Component @RequiredArgsConstructor public class ContractUpdater { private final ContractRepository repository; @Transactional public void updateContractById(Long contractId, Dto contractDto) { Contract contract = repository.findOne(contractId); contract.setValue(contractDto.getValue()); repository.save(contract); } }
Sicher haben Sie schon oft etwas Ähnliches gesehen und geschrieben. Hier gefällt mir nicht, dass die Methode den Status der Entität ändert, ihn aber nicht zurückgibt. Wie verhalten sich ähnliche Framework-Methoden? Beispielsweise geben org.springframework.data.jpa.repository.JpaRepository::save und javax.persistence.EntityManager::merge einen Wert zurück. Angenommen, nach der Aktualisierung eines Vertrags müssen wir ihn außerhalb der update abrufen. Es stellt sich so etwas heraus:
@Transactional public void anotherMethod(Long contractId, Dto contractDto) { updateService.updateContractById(contractId, contractDto); Contract contract = repositoroty.findOne(contractId); doSmth(contract); }
Ja, wir könnten die Entität direkt an die UpdateService::updateContract Methode übergeben, indem wir ihre Signatur ändern. Es ist jedoch besser, dies folgendermaßen zu tun:
@Component @RequiredArgsConstructor public class ContractUpdater { private final ContractRepository repository; @Transactional public Contract updateContractById(Long contractId, Dto contractDto) { Contract contract = repository.findOne(contractId); contract.setValue(contractDto.getValue()); return repository.save(contract); } }
Dies hilft einerseits, den Code zu vereinfachen, andererseits hilft es beim Testen. Im Allgemeinen ist das Testen von void Methoden eine äußerst trostlose Aufgabe, die ich anhand des gleichen Beispiels zeigen werde:
@RunWith(MockitoJUnitRunner.class) public class ContractUpdaterTest { @Mock private ContractRepository repository; @InjectMocks private ContractUpdater updater; @Test public void updateContractById() { Dto dto = new Dto(); dto.setValue("- "); Long contractId = 1L; when(repository.findOne(contractId)).thenReturn(new Contract()); updater.updateContractById(contractId, contractDto);
Aber alles kann einfacher gemacht werden, wenn die Methode einen Wert zurückgibt:
Stellen Sie sicher @RunWith(MockitoJUnitRunner.class) public class ContractUpdaterTest { @Mock private ContractRepository repository; @InjectMocks private ContractUpdater updater; @Test public void updateContractById() { Dto dto = new Dto(); dto.setValue("- "); Long contractId = 1L; when(repository.findOne(contractId)).thenReturn(new Contract()); Contract updated = updater.updateContractById(contractId, contractDto); assertEquals(dto.getValue(), updated.getValue()); } }
Auf einen Schlag wird nicht nur der Aufruf von ContractRepository::save überprüft, sondern auch die Richtigkeit des gespeicherten Werts.
Fahrradbau
Öffnen Sie zum Spaß Ihr Projekt und suchen Sie danach:
lastIndexOf('.')
Mit hoher Wahrscheinlichkeit sieht der gesamte Ausdruck ungefähr so aus:
String fileExtension = fileName.subString(fileName.lastIndexOf('.'));
Davor kann kein statischer Analysator warnen, es geht um das neu erfundene Fahrrad. Meine Herren, wenn Sie ein bestimmtes Problem im Zusammenhang mit dem Dateinamen / der Dateierweiterung oder dem Pfad dazu lösen, genau wie beim Lesen / Schreiben / Kopieren, dann wurde in 9 von 10 Fällen die Aufgabe bereits vor Ihnen gelöst. Binden Sie sich daher an die Fahrradkonstruktion an und nehmen Sie fertige (und bewährte) Lösungen:
import org.apache.commons.io.FilenameUtils;
In diesem Fall sparen Sie Zeit, die für die Überprüfung der Eignung des Fahrrads aufgewendet wird, und erhalten erweiterte Funktionen (siehe Dokumentation FilenameUtils::getExtension ).
Oder hier ist ein Code, der den Inhalt einer Datei in eine andere kopiert:
try { FileChannel sc = new FileInputStream(src).getChannel(); FileChannel dc = new FileOutputStream(new File(targetName)).getChannel(); sc.transferTo(0, sc.size(), dc); dc.close(); sc.close(); } catch (IOException ex) { log.error("", ex); }
Welche Umstände können uns verhindern? Tausende von ihnen:
- Das Ziel kann ein Ordner sein, überhaupt keine Datei
- Die Quelle kann ein Ordner sein
- Quelle und Ziel können dieselbe Datei sein
- Ziel kann nicht erstellt werden
- usw. und so weiter.
Das Traurige ist, dass wir mit der Selbstaufnahme alles lernen, was wir bereits beim Kopieren gelernt haben.
Wenn wir es mit Bedacht tun
import org.apache.commons.io.FileUtils; try { FileUtils.copyFile(src, new File(targetName)); } catch (IOException ex) { log.error("", ex); }
Dann wird ein Teil der Überprüfungen vor dem Beginn des Kopierens durchgeführt, und eine mögliche Ausnahme ist informativer (siehe die Quellen von FileUtils::copyFile ).
Vernachlässigung von @ Nullable / @ NotNull
Angenommen, wir haben eine Entität:
@Entity @Getter public class UserEntity { @Id private Long id; @Column private String email; @Column private String petName; }
In unserem Fall wird die email Spalte in der Tabelle im Gegensatz zu petName als not null beschrieben. Das heißt, wir können die Felder mit den entsprechenden Anmerkungen markieren:
import javax.annotation.Nullable; import javax.annotation.NotNull;
Auf den ersten Blick sieht dies für den Entwickler wie ein Hinweis aus, und das ist es auch. Gleichzeitig sind diese Anmerkungen ein viel leistungsfähigeres Werkzeug als ein normales Etikett.
Entwicklungsumgebungen verstehen sie beispielsweise, und wenn wir nach dem Hinzufügen von Anmerkungen versuchen, dies folgendermaßen zu tun:
boolean checkIfPetBelongsToUser(UserEnity user, String lostPetName) { return user.getPetName().equals(lostPetName); }
dann warnt uns die "Idee" mit dieser Nachricht vor der Gefahr:
Der Methodenaufruf 'equals' kann 'NullPointerException' erzeugen.
Im Code
boolean hasEmail(UserEnity user) { boolean hasEmail = user.getEmail() == null; return hasEmail; }
Es wird eine weitere Warnung geben:
Bedingung 'user.getEmail () == null' ist immer 'false'
Dies hilft dem eingebauten Zensor, mögliche Fehler zu finden und die Codeausführung besser zu verstehen. Aus dem gleichen Grund sind Anmerkungen nützlich, um Methoden zu platzieren, die einen Wert und ihre Argumente zurückgeben.
Wenn meine Argumente nicht schlüssig erscheinen, schauen Sie sich den Quellcode eines ernsthaften Projekts an, denselben "Frühling" - sie sind mit Anmerkungen wie ein Weihnachtsbaum aufgehängt. Und das ist keine Laune, sondern eine ernsthafte Notwendigkeit.
Der einzige Nachteil scheint mir die Notwendigkeit zu sein, die Anmerkungen in einem modernen Zustand ständig aufrechtzuerhalten. Wenn Sie es sich ansehen, ist es eher ein Segen, denn wenn wir immer wieder zum Code zurückkehren, verstehen wir es immer besser.
Nachlässigkeit
Es gibt keine Fehler in diesem Code, aber es gibt einen Überschuss:
Collection<Dto> dtos = getDtos(); Stream.of(1,2,3,4,5) .filter(id -> { List<Integer> ids = dtos.stream().map(Dto::getId).collect(toList()); return ids.contains(id); }) .collect(toList());
Es ist nicht klar, warum eine neue Liste von Schlüsseln erstellt werden soll, mit denen die Suche durchgeführt wird, wenn sich diese beim Durchlaufen des Streams nicht ändert. Es ist gut, dass es nur 5 Elemente gibt, aber was ist, wenn es 100500 davon gibt? Und wenn die getDtos Methode 100500 Objekte zurückgibt (in der Liste!), Welche Leistung hat dieser Code dann? Keine, also ist es besser so:
Collection<Dto> dtos = getDtos(); Set<Integer> ids = dtos.stream().map(Dto::getId).collect(toSet()); Stream.of(1,2,3,4,5) .filter(ids::contains) .collect(toList());
Auch hier:
static <T, Q extends Query> void setParams( Map<String, Collection<T>> paramMap, Set<String> notReplacedParams, Q query) { notReplacedParams.stream() .filter(param -> paramMap.keySet().contains(param)) .forEach(param -> query.setParameter(param, paramMap.get(param))); }
Offensichtlich ist der vom Ausdruck inParameterMap.keySet() Wert unverändert, sodass er in eine Variable verschoben werden kann:
static <T, Q extends Query> void setParams( Map<String, Collection<T>> paramMap, Set<String> notReplacedParams, Q query) { Set<String> params = paramMap.keySet(); notReplacedParams.stream() .filter(params::contains) .forEach(param -> query.setParameter(param, paramMap.get(param))); }
Solche Abschnitte können übrigens mit der Prüfung 'Objektzuordnung in einer Schleife' berechnet werden.
Wenn die statische Analyse machtlos ist
Das achte Java ist längst ausgestorben, aber wir alle lieben Streams. Einige von uns lieben sie so sehr, dass wir sie überall verwenden:
public Optional<EmailAdresses> getUserEmails() { Stream<UserEntity> users = getUsers().stream(); if (users.count() == 0) { return Optional.empty(); } return users.findAny(); }
Wie Sie wissen, ist der Stream frisch, bevor die Beendigung aufgerufen wird. IllegalStateException Sie also erneut auf die users in unserem Code zugreifen, wird eine IllegalStateException .
Statische Analysegeräte wissen noch nicht, wie sie solche Fehler melden sollen. Daher liegt die Verantwortung für das rechtzeitige Erfassen beim Prüfer.
Es scheint mir, dass das Verwenden von Variablen vom Typ Stream sowie das Übergeben von Variablen als Argumente und das Zurückkehren von Methoden wie das Gehen in einem Minenfeld ist. Vielleicht Glück, vielleicht auch nicht. Daher eine einfache Regel: Jedes Auftreten von Stream<T> im Code sollte überprüft werden (aber auf gute Weise sofort ausgeschnitten werden).
Einfache Typen
Viele sind sich sicher, dass es bei boolean , int usw. nur um Leistung geht. Dies ist teilweise richtig, aber außerdem ist der einfache Typ standardmäßig not null . Wenn sich ein Ganzzahlfeld einer Entität auf eine Spalte bezieht, die in der Tabelle als not null deklariert ist, ist es sinnvoll, int anstelle von Integer . Dies ist eine Art Kombination - und der Speicherverbrauch ist geringer, und der Code wird aufgrund der unnötigen Überprüfung auf null vereinfacht.
Das ist alles Denken Sie daran, dass all das nicht die ultimative Wahrheit ist. Denken Sie mit Ihrem eigenen Kopf und gehen Sie die Anwendung von Ratschlägen sinnvoll an.