Codereview meiner ersten Struct

  • Codereview meiner ersten Struct

    Hallo zusammen,

    ich habe meine erste Struct in Swift "fertiggestellt".

    Hat einer Lust sich den Code mal an zu schauen und Verbesserungsvorschläge zu machen.
    Sowohl inhaltlich als auch strukturell, damit ich eine "Musterdatei" habe an der ich mich zukünftig lang hangelnd kann.

    Vielen lieben Dank im Vorraus!
    Dateien

  • So ganz allgemein ist es schwierig, ohne Kontext den Code zu bewerten. Ist ja am Ende des Tages auch viel Geschmacksache und hängt vom Projekt etc. ab.

    Heruhaundo schrieb:

    damit ich eine "Musterdatei" habe an der ich mich zukünftig lang hangelnd kann.
    Funktioniert selten :D Wichtig ist es meiner Meinung nach, einen Sprachen-typischen Syntax / Code Style zu pflegen, sodass man sich a) selbst leichter tut, anderen Code zu lesen, und b) andere sich leichter tun, deine Code zu lesen. Das zu lernen geht meistens am einfachsten, indem man einfach viele Tutorials macht, z.B. auch WWDC Sessions von Apple schaut usw.

    So ganz spontan fällt mir bzgl. Code-Style auf:
    • Ewig lange ----- in den Kommentaren for Funktionen ist eher Bad Practice aus folgenden Gründen: Sie sehen nur bei einer bestimmten Fensterbreite gut aus, sonst hat man einen Linebreak und man hat 5 Zeilen verbraucht für einen "hilfreichen" Kommentar aus einem Wort: "Initializer". Stattdessen würde ich hier a) keine Leerzeile lassen, b) /// oder /**/ verwenden (das hat den Vorteil, dass Xcode dann den Kommentar erkennt und ihn dir überall sonst anzeigt, wenn du mit Alt auf die Funktion / Initializer klickst bzw. in der Autocompletion. C) Ist "Initializer" swiftier als "I N I T I A L I Z E R". ;) Wenn im Kommentar eh kein Mehrwert ist, einfach weglassen.
    • Speaking of Swifty: Es ist eher ungewöhnlich Swift.max, Swift.min oder Swift.zero zu schreiben. Stattdessen einfach die Abkürzung .max, .min, .zero nehmen.
    • Es ist eher üblich, keine Leerzeile vor einem Doppelpunkt bei Variablendeklarationen zu machen, also eher lowerBound: Magnitude anstelle von lowerBound : Magnitude.
    • Ich weiß nicht, ob es irgendeinen Grund hat, weshalb deine ganzen Argumente im Initializer mit einem _ beginnen. Ohne speziellen Grund ist das auch eher untypisch.

  • Osxer schrieb:

    So ganz allgemein ist es schwierig, ohne Kontext den Code zu bewerten. Ist ja am Ende des Tages auch viel Geschmacksache und hängt vom Projekt etc. ab.

    Heruhaundo schrieb:

    damit ich eine "Musterdatei" habe an der ich mich zukünftig lang hangelnd kann.
    Funktioniert selten :D Wichtig ist es meiner Meinung nach, einen Sprachen-typischen Syntax / Code Style zu pflegen, sodass man sich a) selbst leichter tut, anderen Code zu lesen, und b) andere sich leichter tun, deine Code zu lesen. Das zu lernen geht meistens am einfachsten, indem man einfach viele Tutorials macht, z.B. auch WWDC Sessions von Apple schaut usw.
    So ganz spontan fällt mir bzgl. Code-Style auf:
    • Ewig lange ----- in den Kommentaren for Funktionen ist eher Bad Practice aus folgenden Gründen: Sie sehen nur bei einer bestimmten Fensterbreite gut aus, sonst hat man einen Linebreak und man hat 5 Zeilen verbraucht für einen "hilfreichen" Kommentar aus einem Wort: "Initializer". Stattdessen würde ich hier a) keine Leerzeile lassen, b) /// oder /**/ verwenden (das hat den Vorteil, dass Xcode dann den Kommentar erkennt und ihn dir überall sonst anzeigt, wenn du mit Alt auf die Funktion / Initializer klickst bzw. in der Autocompletion. C) Ist "Initializer" swiftier als "I N I T I A L I Z E R". ;) Wenn im Kommentar eh kein Mehrwert ist, einfach weglassen.
    • Speaking of Swifty: Es ist eher ungewöhnlich Swift.max, Swift.min oder Swift.zero zu schreiben. Stattdessen einfach die Abkürzung .max, .min, .zero nehmen.
    • Es ist eher üblich, keine Leerzeile vor einem Doppelpunkt bei Variablendeklarationen zu machen, also eher lowerBound: Magnitude anstelle von lowerBound : Magnitude.
    • Ich weiß nicht, ob es irgendeinen Grund hat, weshalb deine ganzen Argumente im Initializer mit einem _ beginnen. Ohne speziellen Grund ist das auch eher untypisch.

    OK!

  • Heruhaundo schrieb:

    OK!
    • --------- nehme ich raus, ich fand es halt schön übersichtlich.

    Finde ich gerade unglaublich überladen.

    Ich würde persönlich auch solche "Überschriften" wie

    Quellcode

    1. // ––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––
    2. // T Y P E A L I A S
    3. // ––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––


    raus lassen, wenn du danach ganz offensichtlich

    Quellcode

    1. public typealias Magnitude = UInt
    schreibst.. Jeder der weiß was ein Typealias ist, wird sehen.. Da is ein typealias :D

    Ich würde von der Reihenfolge einfach so vorgehen...

    Quellcode

    1. struct xyz{
    3. 1. typealias
    5. 2. static
    7. 3. instanz
    9. ....
    11. }
    Alles anzeigen

    Sehe, auch für "Fremde" keinen Mehrwert solche exorbitant großen Überschriften zu schreiben... Nur für Leute die halt wirklich keine Ahnung haben von Swift ist das vielleicht hilfreich die Syntax zu blicken.. Aber selbst die können ja lesen das es sich um einen typealias, einen static oder eine inizialisierte Variable handelt.

    Ansonsten, dass was Osxer sagt - diese ganzen "// ---- " sind nicht nötig.
    Machen es nur unnötig Überladen, aus meiner Sicht und trainieren nur deine Pfoten beim Cmd + c, Cmd + v drücken.

    Sehr Gut ist allerdings dieser Part...

    Quellcode

    1. /// Returns the remainder of dividing the first value by the second.
    2. ///
    3. /// - Parameters:
    4. /// - lhs : The value to divide.
    5. /// - rhs : The value to divide `lhs` by. `rhs` must not be zero.
    6. public static func % (_lhs : Self, _rhs : Self) -> Self {
    7. return Self.init(_value : Magnitude(_lhs.magnitude % _rhs.magnitude),
    8. _lowerBound : _lhs.lowerBound,
    9. _upperBound : _lhs.upperBound,
    10. _allowedZero : _lhs.isZeroAllowed,
    11. _allowedInfinity : _lhs.isInfinityAllowed,
    12. _increments : _lhs.increments)
    13. }
    Alles anzeigen
    Ich persönlich hätte es leicht abgeändert, aber inhaltlich passt das so.

    So ungefähr schreibe ich meine Funktionsbeschreibungen.

    Quellcode

    1. /*
    2. Returns the remainder of dividing the first value by the second.
    4. Parameters:
    5. lhs : The value to divide.
    6. rhs : The value to divide `lhs` by. `rhs` must not be zero.
    7. */
    8. public static func ....

    Quellcode

    1. /*
    2. struct dient als Beispiel um bla bli blubb.
    4. */
    6. struct LimitedBeispiel{
    7. //Wenn nicht klar ist wofür das typealias ist oder ...
    8. public typealias Magnitude = UInt
    10. //wenn ich zwei Variablen brauch um den Start und Endpunkt zu definieren...
    11. public static let start : int = 0
    12. public static let end : int = 1
    14. //wenn ich eine einzelne benötige Variable habe, die ich ggf. ausgeben will beschreibe ich sie kurz
    15. public var vorname : String = "Bernd"
    16. //so wie du es bereits gemacht hast, kurze Beschreibung...
    17. private var wrappedLowerBound : Magnitude
    18. {
    19. ...bla bla...
    20. }
    22. /*
    23. Beschreibe auch die init Funktion sammt parametern
    25. Param:
    26. a: anfang vom alphabet
    27. b: ende vom alphabet
    28. */
    29. public init(...){...}
    31. }
    Alles anzeigen
    Und sobald man eine Funktion hat, die man wie oben beschreibt und diese Funktion einen Return-value hat, wird der auch angegeben. Also..


    /*
    Funktion abz gibt mir endlich mein C zurück

    Param:
    a: Anfang
    b: Mittel
    z: Ende

    Return:
    c: errechnete Variable C aus kalkulation von a+b -z
    */

  • Heruhaundo schrieb:

    Das mit dem _ meinte ein Dozent in einem Udemy Swift Kurs, um auf die Funktion beschränkte Gültigkeit der Variable "hinzuweisen".
    Das spricht imo stark gegen den Kurs :whistling: … es gibt erschreckend viele Programmierer, die mehr oder weniger willkürlich mit Unterstrichen um sich schmeissen, weil sie die anderswo (bei Apple…) mal gesehen haben.
    Frag dich am besten selbst: Wo ist da der Hinweis auf irgendwas? (historisch wurde das auch eher für Internas verwendet).
    Ein ähnlicher Aberglaube sind Extensions zu einem Typ, der in der selben Datei deklariert wurde — aber dieser Cargo-Kult ist schon so weit verbreitet, dass man da gegen Windmühlen kämpft :-/.

    Ein Tipp zu den Trennstrichen: Probiere mal

    Quellcode

    1. // MARK: -

  • Zugegeben, ich mochte die Extensions noch nie und sah es bislang immer als Notlösung an, um eine bestehende Lib zu vereinfachen.

    Aktuell ertappe ich mich, wie ich aber die verfluchten Extensions im eigenen Code, und was viel schlimmer ist, im gleichen Source file verwende.

    Das mit dem Underscore, ist leider einfach nur beschi**en
    einerseits verwendet es der Assembler um Funktionen eindeutig zu halten, und nun auch noch der Compiler, und das beidem bescheidenen Zeichenvorrat. So hab ich mich wohl oder übel dazu überreden lassen statt einen leading underscore, einen trailing underscore zu verwenden.

    Mit Mittelstrichen bin ich aber noch nie warm geworden …

  • Was genau meint Ihr nun mit den Extension im gleichen Source-File?
    Ich hab die Extensions nur der Übersichtlichkeit halber erstellt.
    Was ist nun Best Practice? Die Extensions in eine eigene Datei auslagern oder garnicht erst als Extension sondern direkt in der Class/Struct schreiben?

  • Ich habe Eure Anregungen hoffentlich korrekt umgesetzt.
    Könnt Ihr bitte noch mal drüber schauen. Auch, ob an sich die Reihenfolge im Sourcecode sinnig ist.
    /// habe ich gelassen, ist bei Apple selber auch so.
    Dateien

  • Das mit den Extensions in der Hauptdatei ist quasi eine Religion: Die Anhänger glauben, dass man damit irgendwelche positiven Effekte erzielt; andere sparen sich Getippe und erreichen den selben Effekt (und mehr) mit Kommentaren…
    Aber es hat halt mal irgendein Influencer geschrieben, dass es total toll ist, wenn man versteckt, welche Protokolle ein Typ erfüllt, und das hat sich ausgebreitet wie der Kult von der heiligen Sandale.
    Inzwischen verdanken wir diesem Ritual auch einen Teil des Chaos bei den Zugriffsrechten (private / fileprivate).

    Traurige Wahrheit ist: In vielen Teams machst du dir das Leben einfacher, wenn du ohne Nachdenken den Schafen folgst — mit Nachdenken und vor allem Hinterfragen macht man sich schnell Ärger.

    Das mit den drei Schrägstrichen ist anscheinend auf dem selben Level angesiedelt … allerdings lässt sich das leicht auflösen:
    Das ist ein Dokumentations-Kommentar; daraus werden auch die Hinweise generiert, die Xcode zu dem kommentierten Konstrukt anzeigen kann.

  • Auch wenn du bei dem Ritual mit den Extensions bleibst:
    Die Kommentare davor würde ich ersatzlos streichen (oder aber mindestens ein "/" je Zeile ;-).

    * Die erste Zeile enthält keinen Informationsgehalt, den man nicht eh schon für den Compiler braucht
    * Copyright-Hinweise braucht man oft überhaupt nicht, und wenn doch reicht einer am Anfang der Datei
    * Wann irgendwas geschrieben wurde ist normalerweise komplett unwichtig — und wenn man die Information doch braucht, holt man sie sich aus dem Versionskontrollsystem; gleiches gilt natürlich für Versionsnummer.

    Unterm Strich sind das nur Daten, die Arbeit machen.

  • Wenn ich mir deinen Code so ansehe, frage ich mich, was macht das Ding eigentlich? Massen an Dokumentation, doch kaum was relevantes und extrem unübersichtlich.

    Ausserdem frage ich mich, weshalb machst du hoer keinen generischen PropertyWrapper und schmeisst neunzig Prozent des Codes weg? Würde der Übersichtlichkeit sicher gut tun.

  • Wolf schrieb:

    Wenn ich mir deinen Code so ansehe, frage ich mich, was macht das Ding eigentlich? Massen an Dokumentation, doch kaum was relevantes und extrem unübersichtlich.

    Ausserdem frage ich mich, weshalb machst du hoer keinen generischen PropertyWrapper und schmeisst neunzig Prozent des Codes weg? Würde der Übersichtlichkeit sicher gut tun.
    Das mit dem PropertyWrapper habe ich versucht aber nicht hin bekommen.
    Meine Kenntnisse sind vermutlich einfach noch zu gering.
    Vielleicht erläutere ich einfach mal was mein Ziel ist.
    • Ich brauche eine positive Ganzzahlen die ich mittels @Published in einem Picker verwenden kann. z.B. für Sekunden
    • Der Wertebereich muss zur Laufzeit mit min und max eingeschränkt werden können. z.B. 1-60
    • In Ausnahmefällen benötige ich aber das Merkmal, dass der Wert unterhalb von min liegen darf. z.B. für sofort
    • In Ausnahmefällen benötige ich aber das Merkmal, dass der Wert oberhalb von max liegen darf. z.B. für nie
    • Und evtl. muss der Wert einem oder mehreren Inkrementen entsprechen. z.B. 5
    So könnte ich dann sehr einfach, so dachte ich zumindest, einen Picker abbilden der z.b. folgende Werte zulässt:
    • sofort
    • 5 Sekunden
    • 10 Sekunden
    • 15 Sekunden
    • 20 Sekunden
    • 25 Sekunden
    • 30 Sekunden
    • 35 Sekunden
    • 40 Sekunden
    • 50 Sekunden
    • 55 Sekunden
    • 1 Minute
    • 2 Minuten
    • 3 Minuten
    • 4 Minuten
    • 5 Minuten
    • niemals
    90% des Codes habe ich ja nur gebraucht, um dem Protokolll zu entsprechen, hätte ich mir auch gerne gespart.

    Vielleicht bin ich auch auf dem Holzweg und das geht viel einfacher.

    Aber unabhängig von dem Problem, geht es mir ja genau darum, wie ich Code in Swift übersichtlich schreibe.

  • Heruhaundo schrieb:

    Ich habe Eure Anregungen hoffentlich korrekt umgesetzt.
    Könnt Ihr bitte noch mal drüber schauen. Auch, ob an sich die Reihenfolge im Sourcecode sinnig ist.
    /// habe ich gelassen, ist bei Apple selber auch so.
    Finde ich übrigens sehr schön und vor allem übersichtlich geworden jetzt :)

    Am Ende darfst du nie vergessen - wenn du es alleine wartest, musst du dich auch am besten zurecht finden.

    Lg