Re: [ccd] Übersicht für clean-code-developer@googlegroups.com - 7 Benachrichtigungen in 1 Thema

[Robin Schürer]

Hallo,

ausgehend von Ralfs Lösung nochmal als Java-Version.

public static String normalizeTopic(String topic) {
   String replaced  = topic.replace(".", "_");
   return ingoreInvalidChars(replaced);
}

private static String ingoreInvalidChars(String topic) {
   return topic
         .chars()
         .mapToObj(i -> (char)i)
         .filter(Application::isValidChar)
         .map(Object::toString)
         .collect(Collectors.joining());
}

private static boolean isValidChar(char ch) {
   if (('A' <= ch) && (ch <= 'Z')) return true;
   if (('a' <= ch) && (ch <= 'z')) return true;
   if (('0' <= ch) && (ch <= '9')) return true;
   if ((ch == '_') || (ch == '-')) return true;
   return false;
}

Am 6. Oktober 2017 um 08:33 schrieb <clean-code...@googlegroups.com>:

clean-code-developer@googlegroups.com

Google Groups

Themen in dieser Übersicht
Alle Themen anzeigen

• Feedback erwünscht - 7 Aktualisierungen

Feedback erwünscht

"Robin Schürer" <robins...@googlemail.com>: Oct 04 11:44PM -0700 Hallo,   ich finde den Methoden-Namen "validateTopic" irgendwie komisch. Vor allem da ein String zurückgegeben wird.   Am Mittwoch, 4. Oktober 2017 08:02:38 UTC+2 schrieb Matthias Kolley:

Matthias Kolley <matthias.kolley@googlemail.com>: Oct 05 02:18AM -0700 Hallo Robin   Erstmal: Danke für dein Feedback. :)   ich finde den Methoden-Namen "validateTopic" irgendwie komisch.     Ich habe überlegt, ob ich den nicht auch schon umbenennen soll, habe es offensichtlich vergessen. Du hast natürlich vollkommen recht. Die Methode müsste eher *convertTopic* oder *validateAndConvertTopic *oder so heißen. Woebi wenn ich die 2. Option so sehe, macht die Methode ja im Prinzip wieder 2 Sachen. Prüfen ob korrekt und wenn nicht dann konvertieren. Es ist also noch Bedarf....   Danke.

Guenter Schwann <g.sc...@gmail.com>: Oct 05 01:52PM +0200 Hi Matthias,   Die neue Lösung ist weit mehr als doppelt so lang. Und in meinen Augen dadurch nicht wirklich besser lesbar.   Ich frage mich ob die Funktionen isUpperCaseChar(), isLowerCaseChar() etc. wiederverwendet werden (können/sollen). Wenn nicht, würde ich persönlich sie weg lassen.   Allgemein sind *mir* hier zu viele if statement. Vor allem in der letzten Funktion. Ein simples return (lowerLimit <= charToCheck) && (charToCheck <= upperLimit); Ist einfacher. Auch in isValidChar() könnte man die Checks zusammenfassen.   Die Dokumentation war auch nicht so schlecht (JavaDoc/Doxygen verwenden). Hätte ich gelassen bzw noch leicht verbessert. Kommentar != Dokumentation.   Und beim Thema Funktionsnamen: isAlreadyValidTopic() - das "already" in dem Namen gibt dem ganzen einen unnötigen zeitlichen Bezug (also ob die Funktion wüsste wann sie aufgerufen wird - weiss sie nicht, sollte sie auch nicht). isValidTopic() oder validTopic() ist kürzer und besser.   Die "perfekte" Lösung gibt es sowieso nicht. Denn da spielt noch viel eigener "Geschmack" mit - der sich im Laufe der Zeit auch stark ändert ;)   Gruß Günter     Am 5. Oktober 2017 um 11:18 schrieb 'Matthias Kolley' via Clean Code   -- Guenter Schwann

Joachim Spehl <j.sp...@googlemail.com>: Oct 05 04:54AM -0700 Bei validate würde ich als Ergebnis eher ein bool oder ein result-Objekt erwarten. Irgendwie macht deine Methode eher sowas, wie eine Normalisierung, oder?   String getNormalizedTopic(String topic)   Am Mittwoch, 4. Oktober 2017 08:02:38 UTC+2 schrieb Matthias Kolley:

Jan Lippert <lippe...@gmail.com>: Oct 05 01:26PM Hi,   in der vorgeschlagenen Lösung wird ein regulärer Ausdruck verwendet. Ähnlich könnte das normalisieren eines Topics umgesetzt werden. In der ursprünglichen Variante gibt es diesen Kommentar: //replaces all "." by "_" //ignores everything that is not a letter, a number, "_" or "-"   Die String-Klasse hat eine Methode replaceAll, welche für eben diese Schritte genutzt werden könnte:   final String INVALID_CHARACTERS_REG_EX = "[^1a-zA-Z0-9_-]"; /** * Replaces all "."s by "_" and removes everything that is not a valid character (letters, numbers, dashes, underscores) of the given topic. */ static String getNormalizedTopic(String topic) { if (topic == null) { /* TODO handle null or mark topic as @NotNull */ return ""; } topic.replaceAll("\\.", "_").replaceAll(INVALID_CHARACTERS_REG_EX, ""); }   Vermutlich kann INVALID_CHARACTERS_REG_EX auch zum validieren benutzt werden. Viele Grüße, Jan   'Joachim Spehl' via Clean Code Developer < clean-code-developer@googlegroups.com> schrieb am Do., 5. Okt. 2017 um 13:54 Uhr:

Matthias Kolley <matthias.kolley@googlemail.com>: Oct 05 06:55AM -0700 Hallo Jan.   Wow Danke. Ja die Methode der String-Klasse ist im Prinzip bekannt. Mein Problem war wahrcheinlich eher die unerfahrenheit mit Reg-Ex.   @Birdy und @Joachim Vielen Dank auch für euer Feedback. Selbst ohne Jan's Lösung konnte ich dadurch a) meine Methoden Anzahl reduzieren und es b) trotzdem lesbar halten. z.B. *validateTopic* in *normalizeTopic* umbenennen. Allerdings ist "meine" Lösung dann immernoch doppelt so lang wie das Original.   Die if's in *isValidChar* würde ich nur ungern in eine Zeile schreiben, weil das (für mich) zu viele Konditionen in einem if sind.   "Meine" Lösung sieht aktuell wie folgt aus: private static final String VALID_TOPIC_REGEX = "[a-zA-z0-9_-]+";   /** * Takes a topic and normalizes it by replacing all "." with a "_". * The normalization ignores everything that is not a letter, number, "_" oder "-". * Therefore everything else will be removed from the given topic. * @param topic the topic to normalize * @return a normalized topic with "." replaced by "_" and everything that is not a letter, number, "_" or "-" removed */ static String normalizeTopic(final String topic) { if (topic.matches(VALID_TOPIC_REGEX)) { return topic; } final StringBuilder sb = new StringBuilder(); for (final char c : topic.toCharArray()) { if (isValidChar(c)) { sb.append(c); } else if (c == '.') { sb.append('_'); } } return sb.toString(); }   private static boolean isValidChar(final char charToCheck) { if (isBeetweenIncluding(charToCheck, 'A', 'Z')) { return true; } if (isBeetweenIncluding(charToCheck, 'a', 'z')) { return true; } if (isBeetweenIncluding(charToCheck, '0', '9')) { return true; } if (charToCheck == '_' || charToCheck == '-') { return true; } return false; }   private static boolean isBeetweenIncluding(final char charToCheck, final char lowerLimit, final char upperLimit) { return lowerLimit <= charToCheck && charToCheck <= upperLimit; }   Wobei die Lösung von Jan natürlich unschlagbar kurz ist!

Ralf Westphal <in...@ralfw.de>: Oct 05 10:45AM -0700 Ich finde, die Lösung stand schon in der ursprünglichen Aufgabe drin. Warum hast du die nicht erhalten? Du hättest sie nur in Code gießen müssen:   public static string normalize_topic(string topic) { topic = Replace_dot(topic); return Ignore_invalid_chars(topic); }   private static string Replace_dot(string topic){ return topic.Replace(".", "_"); }   private static string Ignore_invalid_chars(string topic) { var sb = new StringBuilder(); foreach(var ch in topic) if (Is_valid_char(ch)) sb.Append(ch); return sb.ToString(); }   private static bool Is_valid_char(char ch) { if (('A' <= ch) && (ch <= 'Z')) return true; if (('a' <= ch) && (ch <= 'z')) return true; if (('0' <= ch) && (ch <= '9')) return true; if ((ch == '_') || (ch == '-')) return true; return false; }     - Der Kommentar wird überflüssig, weil das, was darin stand, jetzt im Code steht. - Die Lösung ist kürzer als das Original. - Der Funktionsname passt jetzt auch besser ;-) - Der Prozess der Transformation ist klar ersichtlich. - Die Prüfung auf valide Zeichen ist jetzt fokussiert. Sie tut nur das und fügt nicht auch noch Zeichen zu einer StringBuilder hinzu. - Die Prüfung auf valide Zeichen ist erstmal so gelassen, wie im Original. Aber sie kann auch optimiert werden mit RegEx oder sonstwie. - Ignore_invalid_chars() ist umständlich, aber das ist der Sprache geschuldet. Ich habe zwar C# benutzt, aber kein Linq, um nah deinem Java zu bleiben. Wenn du mal Java Streams anschaust, kannst du da aber noch kürzer werden.   private static string Ignore_invalid_chars(string topic) { var validChars = topic.ToCharArray().Where(Is_valid_char); return new string(validChars.ToArray()); }

Zurück nach oben

Sie erhalten diese Zusammenfassung, weil sie Benachrichtigungen von dieser Gruppe abonniert haben. Sie können Ihre Einstellungen auf der Seite mit den Einstellungen für die Gruppenmitgliedschaft ändern. Wenn Sie sich von dieser Gruppe abmelden und keine E-Mails mehr von dieser Gruppe erhalten möchten, senden Sie eine E-Mail an clean-code-developer+unsub...@googlegroups.com.