Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leere Vorzeichen-Felder? #96

Closed
qa-florian-wende opened this issue Aug 28, 2024 · 7 comments
Closed

Leere Vorzeichen-Felder? #96

qa-florian-wende opened this issue Aug 28, 2024 · 7 comments

Comments

@qa-florian-wende
Copy link

Hallo @oboehm,

wir haben folgendes Problem mit den verschiedenen Vorzeichen-Feldern:
Wir würden die Felder gerne mit einem Leerzeichen befüllen, weil wir ein Altsystem neubauen, das Vorzeichen immer dann weglässt, wenn der zugehörige Betrag 0 ist, und sich das neue System exakt genauso verhalten soll, weil die Empfänger der GDV-Daten sich bereits auf gewisse Eigenheiten unseres Systems eingestellt oder gewisse Sonderfälle sogar explizit gewünscht haben.
Jetzt geht natürlich inhaltlich nichts kaputt, wenn wir für 0 ein Vorzeichen "+" setzen, per se sollte es fachlich kein Problem sein, wenn wir uns im Neubau in diesem speziellen Punkt anders verhalten als das Altsystem, aber allein schon, um unsere Ausgabe leichter mit einer alten Ausgabe für dieselbe Eingabe vergleichen zu können, würden wir gerne die entsprechenden Vorzeichen weglassen.

Den Inhalt des Vorzeichen-Feldes auf " " zu setzen, ist erstmal kein Problem, das Vorzeichen selbst ist ein alphanumerisches Feld, daher kann Feld.setInhalt(String) damit umgehen und für den export kriegen wir über Feld.getInhalt() denselben Inhalt auch wieder raus - so weit, so gut. Ein Problem tritt erst auf, wenn wir den Datensatz einem Datenpaket hinzufügen, weil dann zum Beispiel im Fall von Satzart 0500 die Methode setNachsatzSummenAus0500(Datensatz) aufgerufen wird, die sich die relevanten Felder über datensatz.getFeld(bezeichner, BetragMitVorzeichen.class) holt. In der Methode Satz.getBetragMitVorzeichen(Bezeichner) wird dann der Inhalt des neuen BetragMitVorzeichen-Objekts auf betrag.getInhalt() + vorzeichen.getInhalt() gesetzt und dann knallts, weil der entsprechende Setter kein Vorzeichen sieht und versucht eines hinzuzufügen und dann ist der Wert plötzlich zu lang für das Feld. So landeten wir bei der im ersten Moment etwas kryptischen Exception

java.lang.IllegalArgumentException: Feld BetragInWEGemaessZahlungsartMitVorzeichen: Parameter "0000000000000 +" ist laenger als 14 Zeichen!

Wir sind uns nicht ganz sicher, wie der GDV insgesamt zu leeren Vorzeichen-Feldern steht und inwieweit unser Vorgehen das GDV-Format verletzt, weil die Vorzeichen-Felder auch im Handbuch als "AN" gekennzeichnet sind, daher sollte der Default erstmal ein String aus Leerzeichen sein und man könnte sogar argumentieren, dass man das Feld auf " " setzen müsse, wenn man es nicht nutzt.

Es stehen zwar auch im xml unter dem Tag alleWerteDerTabelle nur "+" und "-" als mögliche Ausprägungen, aber auch bei den anderen "enums" im GDV-Format scheint Leerlassen für nicht genutzte Felder zumindest eine gängige Konvention zu sein.

Vorschlag:
Wir erweitern die Methode Satz.getBetragMitVorzeichen(Bezeichner) so, dass sie mit dem Fall eines leeren Vorzeichens umgehen kann. Etwa:

    private BetragMitVorzeichen getBetragMitVorzeichen(final Bezeichner bezeichner) {
        Betrag betrag = getFeld(bezeichner, Betrag.class);

        Feld vorzeichen = getVorzeichenOf(bezeichner);
        String vorzeichenInhalt = vorzeichen.getInhalt();
        if (StringUtils.isBlank(vorzeichenInhalt)) {
            vorzeichenInhalt = "+";
        }

        BetragMitVorzeichen bmv = new BetragMitVorzeichen(Bezeichner.of(bezeichner.getName() + " mit Vorzeichen"),
                betrag.getAnzahlBytes() + 1, ByteAdresse.of(betrag.getByteAdresse()));
        bmv.setInhalt(betrag.getInhalt() + vorzeichenInhalt);
        return bmv;
    }

In der Klasse BetragMitVorzeichen kann dann alles bleiben wie es ist, bei den entsprechenden Objekten ist es ja wirklich sinnvoll, dass sie tatsächlich ein "echtes Vorzeichen" haben, auch die Validierungslogik würde ich nicht anfassen.

Eine Test-Methode dafür könnte etwa so aussehen:

    @Test
    public void testGetBetragMitLeeremVorzeichen() {
        Satz satz = SatzFactory.getSatz(SatzTyp.of(500));
        satz.setFeld(Bezeichner.SCHADENBEARBEITUNGSKOSTEN_IN_WAEHRUNGSEINHEITEN, "00000001234");
        satz.setFeld(Bezeichner.VORZEICHEN, " ");
        BetragMitVorzeichen betrag = satz.getFeld(Bezeichner.SCHADENBEARBEITUNGSKOSTEN_IN_WAEHRUNGSEINHEITEN, BetragMitVorzeichen.class);
        assertEquals(14, betrag.getAnzahlBytes());
        assertEquals('+', betrag.getVorzeichen());
        assertEquals(" ", satz.getFeld(Bezeichner.VORZEICHEN).getInhalt());
        assertEquals(new BigDecimal("12.34"), betrag.toBigDecimal());
    }

Wenn man es noch etwas strenger machen will, könnte man in getBetragMitVorzeichen auch noch prüfen, ob die Zahl tatsächlich 0 ist:

        ...
        if (StringUtils.isBlank(vorzeichenInhalt)) {
            if (StringUtils.isNotBlank(betrag.getInhalt()) && !StringUtils.repeat('0', betrag.getAnzahlBytes()).equals(betrag.getInhalt())) {
                throw new IllegalStateException("Vorzeichenfeld ist leer fuer " + bezeichner + ", aber Betrag ist nicht 0.");
            }
            vorzeichenInhalt = "+";
        }
        ...

Was meinen Sie dazu?

Viele Grüße
Florian

@oboehm
Copy link
Owner

oboehm commented Aug 29, 2024

Hallo Florian,

das sollte kein Problem sein (ich hoffe, du ist ok?). Die bestehenden Tests sollten nicht davon betroffen sein. Stell einfach einen PR, und ich schaue es mir an, wenn ich aus dem Urlaub zurück bin.

mfg
oli b.

@qa-florian-wende
Copy link
Author

Vielen Dank und noch einen schönen Urlaub!

Gruß Florian

P.S.: du ist okay

@oboehm
Copy link
Owner

oboehm commented Sep 1, 2024

Hallo Florian,

ich hab' jetzt den PR gemerge't und mir näher angeschaut und mir überlegt, ob das auch einfacher geht. Wie wäre es, wenn man einfach ein nicht gesetztes Vorzeichen (Leerzeichen) wie ein "+" behandelt? Evtl. könnte man das noch über den Validierungsmode STRICT als Fehler kennzeichen (falls gewünscht).

@qa-florian-wende
Copy link
Author

Wie meinst Du das? Mit den Änderungen aus dem PR passiert doch genau das: Ein nicht gesetztes Vorzeichen wird wie ein "+" behandelt. Oder meinst Du, dass es auch ins BetragMitVorzeichen-Objekt als Leerzeichen reingeschrieben wird und dann innerhalb dieser Klasse erst als "+" interpretiert wird?

@oboehm
Copy link
Owner

oboehm commented Sep 2, 2024

Ja, aber deine Änderung betrifft nur den Fall, wenn 0 im Betrag drin steht. Ich würde hier einen Schritt weitergehen, und ein Leerzeichen immer zulassen und als "+" interpretieren. Also im Prinzip so, wie dein erster Vorschlag war. D.h. ich würde Zeile 623-625 aus https://github.com/oboehm/gdv.xport/pull/97/files wieder entfernen, in der eine IllegalStateException geworfen wird.

@qa-florian-wende
Copy link
Author

Achso, ja, das halte ich auch für sinnvoll, mathematisch ist ja Plus der Default und man kann das Vorzeichen bei nicht-negativen Zahlen weglassen, insofern wäre das m.E. auch im GDV-Format das intuitive Verhalten, zumal es anhand des GDV-Handbuchs nicht verboten zu sein scheint, das Feld leer zu lassen.

@oboehm
Copy link
Owner

oboehm commented Sep 6, 2024

mit 7.2.1 ausgerollt

@oboehm oboehm closed this as completed Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants