SatzTest#hasSparteAsProdukt() fehlerhaft ??

[Klausus01]

Im Test wird ein Satz erzeugt mit
Satz bausparen = SatzRegistry.getInstance().getSatz(SatzTyp.of("0210.580"));
Für diesen Satz gilt:
bausparen instanceof Datensatz == true

Und weil Datensatz.java die von Satz.java geerbte Methode hasDatensatz() überschreibt, wird mit
bausparen.hasSparte()
in Wahrheit die Methode Datensatz#hasSparte()
gerufen. Diese Methode schaut bei auf die Member
Datensatz.sparte
Sie ist bei der oben benutzen Satzerzeugung immer mit dem 2.Teil von SatzTyp besetzt sofern ein 2.Teil vorhanden ist.

Analoges gilt auch für bausparen.getSparte().

ERGO:
der Test belegt weder die Korrektheit von
Satz#hasSparte() noch von Satz#getSparte() .

Würde dieser Test angewendet auf einen Teildatensatz, müsste er z.B. lauten:

Satz bausparen =  SATZ_REGISTRY.getSatz(SatzTyp.of("0210.580")).getTeildatensatz(1);
assertFalse(bausparen.hasSparte()); 
assertEquals(580, bausparen.getSparte());

Das zeigt auf, dass in Satz#hasSparte() und Satz#getSparte() unterschiedlich agiert wird
und führt zur Frage, wie diese beiden Methoden zu verstehen sind.

Wenn sie nur technischen Belangen dienen sollen, schlage ich als Änderung in Satz.java vor:

 public boolean hasSparte() {
	  ByteAdresse adresseSparte = ByteAdresse.of(11);
	  if (hasFeld(adresseSparte)) {
		  Bezeichner adresse11 = getFeld(adresseSparte).getBezeichner();
		  return adresse11.equals(Bezeichner.of("Sparte") || adresse11.equals(Bezeichner.of("Produkt");
	  } else {
		  return false;
	  }
  }

bzw.

public int getSparte() {
	NumFeld sparte = (NumFeld) this.getFeld(ByteAdresse.of(11));
	if (!sparte.getBezeichnung().equals("Sparte") && !sparte.getBezeichnung().equals("Produkt"))
		throw new IllegalArgumentException(
				this.toShortString() + " hat kein Feld \"Sparte\" oder \"Produkt\" an Pos 11!");

	return sparte.toInt();
}

Wenn sie fachlichen Belangen dienen sollen, schlage ich als Änderung in Satz.java vor:

 public boolean hasSparte() {
	  ByteAdresse adresseSparte = ByteAdresse.of(11);
	  if (hasFeld(adresseSparte)) {
		  Bezeichner adresse11 = getFeld(adresseSparte).getBezeichner();
		  return adresse11.equals(Bezeichner.of("Sparte");
	  } else {
		  return false;
	  }
  }

bzw.

public int getSparte() {
	NumFeld sparte = (NumFeld) this.getFeld(ByteAdresse.of(11));
	if (!sparte.getBezeichnung().equals("Sparte"))
		throw new IllegalArgumentException(
			this.toShortString() + " hat kein Feld \"Sparte\" an Pos 11!");

	return sparte.toInt();
}

Ergänzung 17.03.2024:
Die Durchsicht der gesamten Lib hat gezeigt, dass Satz#hasSparte() und Satz#getSparte() immer nur verwendet
werden, um letztlich eine Info über den Aufbau der Satzstruktur zu erhalten, d.h. über den GdvSatzartNamen z.B "0100", "0200", "0220.040", "0220.010.13.1". Damit dienen diese Methoden allein technischen Zwecken.
Beispiele:
für Satzart 0220.010.13.1 gilt: Satz.hasSparte() == true; (der GdvSatzartName hat mindestens 1 Punkt im Namen)
für Satzart 0220.010.13.1 gilt: Satz.getSparte() == 10; (der GdvSatzartName gilt für eine Leben-Struktur)
für Satzart 0100 gilt: Satz.hasSparte() == false; (der GdvSatzartName hat keinen Punkt im Namen)
für Satzart 0100 gilt: Satz.getSparte() nicht erlaubt wg Satz.hasSparte() == false;

HIer ist jedoch zu beachten, dass bei allen Satzarten mit Satz.hasSparte() == false; sowie bei SA0210.000, SA0210.000, SA0220.000 und SA0221.000 dennoch an der ByteAdresse 011 ein Feld mit wohldefinierten Werten ungleich "000"
vorhanden sein kann!

CAVE: Satz.getSparte() ist nicht zwingend identisch mit dem Inhalt zu Satz.getFeld(ByteAdresse.of(11)).getInhalt() .
Beispel: Für Satzart 0220.080 gilt Satz.getSparte() == 80 . Da aber diese Satzart für die Sparten 80, 81,
82, 83, 89, 90, 99, 100, 109, 120, 123, 124, 150, 210, 230, 231 verwendet wird (siehe SatzTyp,java), kann das Feld an
ByteAdresse 011 alle diese Werte beinhalten,

Fazit:
Eine neue Methode SatzTest#hasSparteAsProdukt() ist unnötig. Auch ist es in den besprochenen Methoden nicht relevant, welche Bezeichnung das Feld an Adresse 011 hat. Deshalb empfehle ich folgende Änderung in Satz.java:

/**
 * @Oli: Diese Methode prueft, ob es im GdvSatzartNamen einen 2.Teil gibt (siehe
 *       {@link SatzTyp} )
 *       <p>
 * 
 * 
 *       Prueft, ob der GdvSatzartName <br/>
 *       (siehe "http://www.gdv-online.de/vuvm/best_download.htm") <br/>
 *       einen Spartenteil besitzt. Grundsaetzlich hat jeder GDV-konforme Satz
 *       an ByteAdresse 011 ein "Sparte"/("Produkt")-<b>Feld</b> mit einem
 *       fachlich konkreten Inhalt!
 *
 * @return true, falls der GdvSatzartName einen Spartenteil hat, sonst false.
 * 
 * @since 0.9
 */
public boolean hasSparte() {
	   return StringUtils.isNotEmpty(this.gdvSatzartName) && this.getSatzTyp().hasSparte();
}

/**
 * @Oli: Diese Methode bezieht sich implizit auf die ByteAdresse 011 in den
 *       Kopfdaten! Doch weil die Feldbezeichnung "Sparte" in SA0100, TD5
 *       mehrdeutig ist, muss hier mit der ByteAdresse 011 gelesen werden!
 *       <p>
 * 
 *       Liefert den Spartenteil des GdvSatzartNamens <br/>
 *       (siehe "http://www.gdv-online.de/vuvm/best_download.htm"). <br/>
 *       Vorher sollte allerdings mittels {@link #hasSparte()} geprueft werden,
 *       ob der GdvSatzartName einen Spartenteil besitzt.
 *       
 *       Ansonsten wird ggfs. eine {@link ValidationException} geworfen.
 *       
 * @return der Spartenteil des GdvSatzartNamens
 * 
 * @throws ValidationException falls GdvSatzartName ohne Spartenteil
 * @since 0.9
 */
@JsonIgnore
public int getSparte() {
	if (this.hasSparte()) {
		return SatzTyp.of(this.gdvSatzartName).getSparte();
	}
	
	throw new ValidationException(
			String.format("Der GdvSatzartName '%s' enthaelt keinen Spartenteil!", getGdvSatzartName()));
}

Mit diesen Methoden sind alle Tests grün.

[oboehm]

Das Problem hier ist die Doppeldeutigkeit von getSparte(). Ursprünglich war diese Methode vorgesehen, die Sparte zu bestimmen. Diese Funktionalität hat jetzt aber der SatzTyp übernommen, die eine getSparte()/hasSparte()-Methode hat. Abgesehen davon ist getSparte() jetzt überarbeitet, sodass folgende Unit-Test grün ist:

    @Test
    public void testHasSparteAsProdukt() {
        SatzTyp satzTyp = SatzTyp.of("0210.580");
        Satz bausparen = SatzRegistry.getInstance().getSatz(satzTyp);
        assertTrue(bausparen.hasSparte());
        assertEquals(580, bausparen.getSparte());
        assertEquals(satzTyp, bausparen.getSatzTyp());
        for (Teildatensatz tds : bausparen.getTeildatensaetze()) {
            assertTrue(tds.hasSparte());
            assertEquals(580, tds.getSparte());
            assertEquals(satzTyp, tds.getSatzTyp());
        }
    }

Statt getSparte() und hasSparte() sollte man lieber über den SatzTyp gehen. Will man dagegen tatsächlich das Feld "Sparte", so gibt es jetzt eine Methode getFeldSparte(), mit der das Feld (als Optional) zurückgegeben wird.

[Klausus01]

Bitte Vorsicht bei der Verwendung der Formulierung "... Diese Funktionalität hat jetzt aber der SatzTyp übernommen."
Genau betrachtet, dient der SatzTyp nicht dazu, die fachliche Antwort nach der Sparte zu liefern. SatzTyp liefert die Antwort, ob eine GDV-Satzstruktur für eine spezielle Sparte dient. Dabei ist SatzTyp eng angelehnt an den Begriff "Satzart", wie er auf "gdv-online" verwendet wird:
Beispiel 1: http://www.gdv-online.de/vuvm/bestand/rel2023/ds0210.000.htm ,Hier lautet die GDV-Satzart "0210.000" .
Beispiel 2: http://www.gdv-online.de/vuvm/bestand/rel2023/ds0220.580.01.htm . Hier lautet die GDV-Satzart "0220.580.01".
Die Methode SatzTyp#hasSparte() sagt nun aus, ob die GDV-Satzart einen Spartenteil besitzt.
Das sind die 3 Ziffern nach dem 1. Punkt in der GDV-Satzart. Hier gilt: SatzTyp.hasSparte() == true

Anders sieht es z.B hier aus: http://www.gdv-online.de/vuvm/bestand/rel2023/ds0100.htm . Das ist die GDV-Satzart "0100". Hier gibt es keinen Spartenteil in der GDV-Satzart. Deshalb gilt natürlich hier: SatzTyp.hasSparte() == false.

Dennoch: jede GDV-Struktur hat unabhängig vom Wert von SatzTyp.hasSparte() ein 3-stelliges numerisches Feld an Position 011. Einzige Ausnahmen sind Satzart 0000 (Vorsatz) und Satzart 9999 (Nachsatz). Insgesamt gibt es 159 GDV-Satzarten mit einem 3-stelligen numerischen Feld an Position 011.
Von der insgesamt 159 Satzarten gilt für 134 Satzarten: SatzTyp.hasSparte() == true.

Dabei entspricht der fachliche Inhalt des Feldes an Pos 011 bei 112 GDV-Satzarten dem Wert von SatzTyp.getSparte().
Bei 22 GDV-Satzarten kann der fachliche Inhalt des Feldes an Pos 011 vom Wert von SatzTyp.getSparte() abweichen.
Weitere Infos dazu siehe Javadoc in SatzTyp.java.

Festzuhalten bleibt:
der SatzTyp liefert Informationen über die GDV-Satzart / Satzstruktur.
Satz.getFeldSparte() liefert den fachlichen Inhalt des Feldes an Pos 011,

Jedoch hat SatzTyp#getSparte() noch ein kleines Problem, wenn SatzTyp keinen Spartenanteil hat, z.B. Satzart 0100, 0200, 0300 u.a. Wäre hier nicht eine ValidateException angemessen wie in meinem Vorschlag von Satz#getSparte() (s.o.)?

Konsequenterweise sollte auch die Methode Satz.hasSparte() umgestellt werden auf:

public boolean hasSparte() {
	return this.getSatzTyp().hasSparte();
}

3 Fragen zu getFeldSparte().

1. Macht ein Optional nicht nur Sinn bei einem Satz, der kein Spartenfeld hat (Vor- bzw. Nachsatz)?
2. Jeder andere GDV-konforme Satz hat an Pos 11 ein Spartenfeld. Braucht es diese Methode wirklich, wenn getFeld(ByteAdresse.of(011)) das gleiche Feld liefert? Bei frei definierten Satzarten (0800 - 0899) werden die Karten ohnehin individuell gemacht.
3. In SA0100, Teildatensatz 5 gibt es 2 Felder mit dem Namen "Sparte": Pos 011 und Pos 150! Welches Feld wird von getFeldSparte() geliefert? Wird hier nicht erneut eine Doppeldeutigkeit erzeugt?
   Vorschlag: entweder getSparteFeld() umnennen in getSparteFeld011() oder besser auf getSparteFeld() verzichten (siehe. 2.))

[oboehm]

Sowohl getSparte() und hasSparte() gibt es schon mit v0.9 seit 2013. getSparte() liefert dabei den Inhalt des ersten Felds mit Bezeichner "Sparte" (Byte-Adresse 11). Existiert das Feld mehrfach, liefert es das erste Feld zurück.

Zu den Fragen:

1. Da sowohl Datensatz, als auch Vorsatz und Nachsatz von Satz abgeleitet sind, macht Optional dahingehend Sinn, dass man damit die alte getSparte() und hasSparte()-Funktionalität abbilden kann. Der Name getFeldSparte() ist zudem aussagekräftiger als getSparte() / hasSparte() und kann weniger zu Fehlinterpretation führen.
2. getFeld(ByteAdesse.of(011) würde das Feld mit Byte-Adresse 9 zurückgeben, da Konstanten mit führender 0 als Oktal-Konstanten betrachet werden. Unabhängig davon ist getFeldSparte() lesbarer, da man auch ohne Kenntnisse der Byte-Adresse weiß, welches Feld gemeint ist.
3. getFeldSparte() liefert immer Feld an Byte-Adresse 11 zurück. Möchte man diese Mehrdeutigkeit auflösen, bleibt der Weg über getFeld(ByteAdresse)