Sample-Sourcen ?

[lundefugl]

Hallo Christian

leider hatte ich damals keine Zeit, um eine Erweiterung für die Flightradar24 CSVs zu machen (siehe https://forum.routeconverter.com/thread-...l#pid17103).
Das habe ich irgendwie dann aus den Augen verloren, bis ich dieses Jahr wieder vor dem Problem stand.

Jetzt habe ich mich also mal hingesetzt, um das einzubauen.

Worüber ich nun gestolpert bin. Es gibt zwar für die ganzen verschiedenen Formate Unittests, aber irgendwie werden dafür bei mir lokal die Sample-Dateien nicht gefunden.
Kannst du mir evtl. verraten, wie ich die bekomme ? Ist da etwas zu beachten, wenn man da dann neue Samples ablegen möchte ? 

Und dann ist mir jetzt beim Testen wieder was aufgefallen. Ist es wirklich gewollt, dass man beim Editieren eines Punktes die Zeit in UTC eingeben muss ? Die Anzeige erfolgt ja in der vom Benutzer eingestellten Zeitzone. Im Moment hat man dadurch auch den Effekt, dass ein reiner Doppelklick in eine Zelle den Wert verändert, obwohl man nichts ändert.
Das passiert bei Datumswerten genauso, wie bei Zeitwerten. Beim Datum passiert es, weil da intern mit 00:00:00 als Zeit gearbeitet wird und das dann durch Umrechnung auch auf den Vortag springt. Da muss man folglich den Folgetag eingeben, damit man den gewünschten Tag erhält.

Danke & Gruß
Thomas

[lundefugl]

Noch ein Nachtrag zu dem Editierproblem...

Ich wollte für PositionsModelImpl den Unittest erweitern, um das Verhalten mal etwas testen und reproduzieren zu können. Leider geht das nicht so einfach, da für die benutzerdefinierte Zeitzone eine hochgefahrene Application gebraucht wird, damit RouteConverter.getInstance() nicht null zurückgibt. Hast du sowas schon mal in Tests gemacht ?

[routeconverter]

Hallo Thomas,

schön mal wieder etwas von Dir zu lesen.

Worüber ich nun gestolpert bin. Es gibt zwar für die ganzen verschiedenen Formate Unittests, aber irgendwie werden dafür bei mir lokal die Sample-Dateien nicht gefunden.
Kannst du mir evtl. verraten, wie ich die bekomme ?

Die Beispiele habe ich in einem privaten Repository liegen. Die kann ich nicht veröffentlichen, weil sie mir zumindest teilweise von Nutzern zugesandt wurden.

Ist da etwas zu beachten, wenn man da dann neue Samples ablegen möchte ? 

Ich habe die internen Beispiele bei mir über die Verzeichnisse "drüber" gelinkt:

RouteConverter/navigation-formats-samples/src/samples/
RouteConverter/navigation-formats-samples/src/unrecognized/
RouteConverter/navigation-formats-samples/src/falsedetects/

Die Namen der Verzeichnisse sollten selbstsprechend sein.

Wenn Du für ein neues Format Testdateien benötigst, dann würde ich die in samples/... ablegen und mir senden, damit ich die beim Testlauf auch verwenden kann.

Und dann ist mir jetzt beim Testen wieder was aufgefallen. Ist es wirklich gewollt, dass man beim Editieren eines Punktes die Zeit in UTC eingeben muss ? Die Anzeige erfolgt ja in der vom Benutzer eingestellten Zeitzone.

Schwierige Frage... erfahrene Benutzer wissen, dass üblicherweise UTC verwendet wird. Unerfahrene erwarten implizit ihre eigene Zeitzone, daher wird das auch standardmässig angezeigt. Allerdings ist es wohl konsistenter, in der eingestellten Zeitzone auch Eingaben entgegenzunehmen und ggfs. nach UTC zu wandeln.

[routeconverter]

Noch ein Nachtrag zu dem Editierproblem...

Ich wollte für PositionsModelImpl den Unittest erweitern, um das Verhalten mal etwas testen und reproduzieren zu können. Leider geht das nicht so einfach, da für die benutzerdefinierte Zeitzone eine hochgefahrene Application gebraucht wird, damit RouteConverter.getInstance() nicht null zurückgibt. Hast du sowas schon mal in Tests gemacht ?

Uups, ja das ist natürlich Blödsinn, da müsste eine Funktion hineingereicht werden – kommt wegen der editCell(...) Logik vom TableModel. Ich behebe das und melde mich:

private List<DegreeFormat> getDegreeFormats() {
    DegreeFormat preferred = RouteConverter.getInstance().getUnitSystemModel().getDegreeFormat();
    return DegreeFormat.getDegreeFormatsWithPreferredDegreeFormat(preferred);
}

[lundefugl]

Hallo Christian

danke für die Informationen.

Dann werde ich das mit dem Sample entsprechend mal versuchen umzusetzen.

Bezüglich der Zeit-Editierung und UTC kann man sich sicherlich bezüglich der Eingabe streiten, was da erfahrende Benutzer erwarten oder nicht.
Beim Datum finde ich es aber extrem komisch, dass man da einen anderen Tag eingeben muss. Und was auch irgendwie unschön ist, wenn man sich nur verklickt (also die falsche Zelle zum editieren selektiert), sich der Wert dadurch sofort ändert.

Wenn du da was angepasst hast, dass man das als Unittest auch laufen lassen kann, mache ich dazu mal einen Vorschlag. Du kannst ja dann entscheiden, ob du das übernehmen willst oder nicht.

Gruß
Thomas

[routeconverter]

Beim Datum finde ich es aber extrem komisch, dass man da einen anderen Tag eingeben muss.

Ich stimme Dir zu. Und auch, dass man das Datum absolut korrekt eingeben muss. Und in UTC.

Und was auch irgendwie unschön ist, wenn man sich nur verklickt (also die falsche Zelle zum editieren selektiert), sich der Wert dadurch sofort ändert.

Stimmt.

Wenn du da was angepasst hast, dass man das als Unittest auch laufen lassen kann, mache ich dazu mal einen Vorschlag. Du kannst ja dann entscheiden, ob du das übernehmen willst oder nicht.

Gerne. Ich habe gerade die implizite Verbindung von PositionModelImpl zu RouteConverter#getInstance aufgelöst. Eigentlich müssten UnitTests jetzt leichter sein, bitte probier's mal aus.

[lundefugl]

Hallo Christian

danke für die Anpassung. Ich denke, dass ich mir das morgen mal anschauen kann.
Bezüglich der CSVs von Flightradar24 habe ich mal einen Pullrequest erstellt. Ich habe auch 2 Kommentare angefügt, warum ich 2 Bestandsfunktionen etwas angepasst habe.
Über alles kann man natürlich diskutieren.

Anbei sind die besprochenen Samples. Ich denke, dass das keine geheimen Files sind und das Forum ist ja auch nur für registrierte User lesbar. Insofern sollte das kein Problem sein, dass ich es hier poste. Den Inhalt vom Zip einfach ins Sample-Verzeichnis legen.

Gruß
Thomas


  flightradar24-samples.zip (Size: 236.53 KB / Downloads: 13)

[lundefugl]

Hallo Christian,

ich habe mir deine Änderung angeschaut. Leider löst sie nicht das Problem für die Unittests.
Das Problem ist, dass das PositionsModelImpl dann weiter auf den PositionHelper durchgreift. Und darin wird dann wieder RouteConverter.getInstance() gemacht.

Code:

java.lang.NullPointerException: Cannot invoke "slash.navigation.converter.gui.RouteConverter.getTimeZone()" because the return value of "slash.navigation.converter.gui.RouteConverter.getInstance()" is null

at slash.navigation.converter.gui.helpers.PositionHelper.getDateFormat(PositionHelper.java:180)
at slash.navigation.converter.gui.helpers.PositionHelper.parseDate(PositionHelper.java:200)
at slash.navigation.converter.gui.models.PositionsModelImpl.parseDate(PositionsModelImpl.java:322)
at slash.navigation.converter.gui.models.PositionsModelImpl.editCell(PositionsModelImpl.java:221)
at slash.navigation.converter.gui.models.PositionsModelImpl.edit(PositionsModelImpl.java:203)
at slash.navigation.converter.gui.models.PositionsModelImpl.setValueAt(PositionsModelImpl.java:192)
at slash.navigation.converter.gui.models.PositionsModelGpxTest.testTimeEdit(PositionsModelGpxTest.java:105)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)

In meinen Augen müsste man die Haltung der globalen Modelle, die nichts direkt mit der GUI-Anwendung zu tun haben, von der RouteConverter-Klasse trennen.
Am besten wäre vermutlich, wenn man das in einem eigenen Singleton (z.B. RouteConverterModels) bündelt, auf dem man dann die Modelle per Set-Methoden auch setzen kann. Bei den Aufrufern würde sich dann lediglich RouteConverter.getInstance() auf RouteConverterModels .getInstance() ändern.
 
Ich bin zwar kein Freund von solchen Singleton-Lösungen und die aktuellen Probleme zeigen auch schön den Grund, aber es würde im Moment wahrscheinlich die einfachste Variante sein, ohne die Gesamtarchitektur vom RouteConverter zu ändern. Das kann man dann auch schrittweise umbauen. D.h. im ersten Schritt würden nur die Modelle wechseln, die für den aktuellen Fall gebraucht werden.

Was denkst du dazu ?

Gruß
Thomas

[routeconverter]

Das würde ja einen Singleton durch einen anderen, kleinere ersetzen. Irgendwie unschön. Vielleicht müsste man die Logik hinter editCell() herauslösen. Ich schaue mir das an.

[lundefugl]

Hallo Christian,

zuerst mal ein gutes neues Jahr.

Deine Einschätzung ist richtig. Das Hauptproblem ist ja, dass man für den Unittest nur einen kleinen Teil davon braucht. Und das ist nur der Datenteil. Alles, was mit der Swing-Anwendung selbst zu tun hat, das braucht man eigentlich nicht. Die Trennung ginge etwas Richtung MVC-Pattern. Und den reinen Datenteil im Test hochzufahren, geht vermutlich einfacher.

Allerdings würde ich das bei uns in der Firma auch nicht als gutes Design durchgehen lassen (wegen der komplexen Singletons). Das eigentliche Problem ist, dass die statischen Methoden in PositionHelper auf das aktuelle Singleton durchgreifen. Und da kann man nicht einfach was anderes setzen.

Was man da auch machen könnte ist, dass man den PositionHelper in ein Singleton oder sogar zu einer ganz normalen Klasse umwandelt und dann keine statischen Methoden mehr hat. In Klassen, die den benutzen wolle, die können den PositionHelper sich im Konstruktor bestimmen bzw. erzeugen und benutzen bei den Aufrufen dann diese Instanz. Gleichzeitig gibt es zusätzliche Konstruktoren für die Unittests, bei dem man den PositionHelper auch von aussen vorgeben kann bzw. die benötigten Modelle alternativ vorgeben kann.

Hier könnte man auch ein Interface definieren, die die RouteConverter-Klasse implementiert. Beim Test gibt man dann eine eigene Variante von dem Interface durch, wo man nur den benötigten Teil mit Leben füllt.

In PositionHelper sähe das so aus:

Code:

PositionHelper() {
   this(RouterConverter.getInstane());
}

PositionHelper(RouteConverterIfc routeConverter) {
   this.routeConverter = routeConverter;
}

Und den PositionHelper kann man dann über deine Callback-Klasse in das PositionsModelImpl durchreichen.

Ich hoffe, dass du mir etwas folgen konntest. Solche Dinge nur in einem Chat zu diskutieren, mache ich sonst nicht. Da machen wir meist kurze Calls bei uns in der Firma zu. Da kann man dann auch die ganzen Vor- und Nachteile der verschiedenen Lösungen besser diskutieren.

Gruß
Thomas