From ac545ecdaaf953201144b374ff00950ac7716b2d Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 8 Apr 2026 13:25:06 +0200 Subject: [PATCH] refactor: address PR review concerns - Remove Architekt from WORD_PREFIXES (classifier handles it) - Use Objects.equals for null-safe firstName/lastName comparison - Remove unused trimmed variable in PersonTypeClassifier - Fix containsWord to loop through all occurrences (finds "Eltern" in "Nachbareltern Eltern") - Extract DisplayNameFormatter utility shared by Person and PersonSummaryDTO to eliminate display logic duplication Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/dto/PersonSummaryDTO.java | 7 ++----- .../model/DisplayNameFormatter.java | 12 ++++++++++++ .../raddatz/familienarchiv/model/Person.java | 6 +----- .../service/PersonNameParser.java | 5 +++-- .../service/PersonTypeClassifier.java | 19 +++++++++++-------- .../service/PersonTypeClassifierTest.java | 5 +++++ 6 files changed, 34 insertions(+), 20 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/model/DisplayNameFormatter.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonSummaryDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonSummaryDTO.java index edb448f8..1f71045a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonSummaryDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonSummaryDTO.java @@ -20,10 +20,7 @@ public interface PersonSummaryDTO { long getDocumentCount(); default String getDisplayName() { - StringBuilder sb = new StringBuilder(); - if (getTitle() != null) sb.append(getTitle()).append(" "); - if (getFirstName() != null) sb.append(getFirstName()).append(" "); - sb.append(getLastName()); - return sb.toString().trim(); + return org.raddatz.familienarchiv.model.DisplayNameFormatter.format( + getTitle(), getFirstName(), getLastName()); } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/DisplayNameFormatter.java b/backend/src/main/java/org/raddatz/familienarchiv/model/DisplayNameFormatter.java new file mode 100644 index 00000000..8ec42a85 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/DisplayNameFormatter.java @@ -0,0 +1,12 @@ +package org.raddatz.familienarchiv.model; + +public class DisplayNameFormatter { + + public static String format(String title, String firstName, String lastName) { + StringBuilder sb = new StringBuilder(); + if (title != null) sb.append(title).append(" "); + if (firstName != null) sb.append(firstName).append(" "); + sb.append(lastName); + return sb.toString().trim(); + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/Person.java b/backend/src/main/java/org/raddatz/familienarchiv/model/Person.java index b359fd7b..e731a78a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/Person.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/Person.java @@ -58,10 +58,6 @@ public class Person { @Transient @Schema(accessMode = Schema.AccessMode.READ_ONLY, requiredMode = Schema.RequiredMode.REQUIRED) public String getDisplayName() { - StringBuilder sb = new StringBuilder(); - if (title != null) sb.append(title).append(" "); - if (firstName != null) sb.append(firstName).append(" "); - sb.append(lastName); - return sb.toString().trim(); + return DisplayNameFormatter.format(title, firstName, lastName); } } \ No newline at end of file diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonNameParser.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonNameParser.java index ebd914dd..caa0cad9 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonNameParser.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonNameParser.java @@ -3,6 +3,7 @@ package org.raddatz.familienarchiv.service; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -159,7 +160,7 @@ public class PersonNameParser { if ("?".equals(lastName) && !cleaned.contains(" ")) { lastName = firstName; firstName = null; - } else if (firstName.equals(lastName)) { + } else if (Objects.equals(firstName, lastName)) { firstName = null; } } @@ -217,7 +218,7 @@ public class PersonNameParser { "Frau", "Herr", "Freifrau", "Freiherr", "Tante", "Onkel", "Schwester", "Bruder", "Cousine", "Cousin", "Freundin", "Freund", - "Mutter", "Vater", "Pastor", "Architekt"); + "Mutter", "Vater", "Pastor"); /** Strips known title/relationship prefixes, looping for stacked titles. */ public static TitleResult stripTitle(String input) { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonTypeClassifier.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonTypeClassifier.java index e39abb05..b3b5c094 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonTypeClassifier.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonTypeClassifier.java @@ -24,8 +24,7 @@ public class PersonTypeClassifier { public static PersonType classify(String rawName) { if (rawName == null || rawName.isBlank()) return PersonType.PERSON; - String trimmed = rawName.trim(); - String lower = trimmed.toLowerCase(); + String lower = rawName.trim().toLowerCase(); for (String keyword : SKIP_KEYWORDS) { if (lower.startsWith(keyword.toLowerCase())) return PersonType.SKIP; @@ -50,11 +49,15 @@ public class PersonTypeClassifier { } private static boolean containsWord(String text, String word) { - int idx = text.indexOf(word); - if (idx < 0) return false; - boolean startOk = idx == 0 || !Character.isLetter(text.charAt(idx - 1)); - int end = idx + word.length(); - boolean endOk = end >= text.length() || !Character.isLetter(text.charAt(end)); - return startOk && endOk; + int fromIndex = 0; + while (true) { + int idx = text.indexOf(word, fromIndex); + if (idx < 0) return false; + boolean startOk = idx == 0 || !Character.isLetter(text.charAt(idx - 1)); + int end = idx + word.length(); + boolean endOk = end >= text.length() || !Character.isLetter(text.charAt(end)); + if (startOk && endOk) return true; + fromIndex = idx + 1; + } } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonTypeClassifierTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonTypeClassifierTest.java index ed9132ef..20746379 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonTypeClassifierTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonTypeClassifierTest.java @@ -86,4 +86,9 @@ class PersonTypeClassifierTest { void classify_caseInsensitive() { assertThat(PersonTypeClassifier.classify("firma auschrath")).isEqualTo(PersonType.INSTITUTION); } + + @Test + void classify_containsWord_findsSecondOccurrence() { + assertThat(PersonTypeClassifier.classify("Nachbareltern Eltern")).isEqualTo(PersonType.GROUP); + } }