So, kommen wir zum dritten Teil. Zu Versuchs und Lernzwecken habe ich meine eigene cpl::optional Klasse geschrieben, die genau das macht, was ich zur Zeit brauche. Ich mache mir absichtlich das Leben einfach und vordere, dass der Typ default constructible sein muss. Hält cpl::optional kein Wert, so ist ein default Wert gespeichert. Das hat die schon beschriebenen Nachteile, aber man muss ja pragmatisch denken. Die Syntax soll nicht so einfach wie möglich sein, sondern so einfach, wie sie vorher schon war. Zum Vergleich zeige ich den alten Code, eine funktionale Version und eine neue Version
Alter Code:
bool hasDMP = false;
Punkt DMP;
bool hasSMP = false;
Punkt SMP;
for(const Punkt& punkt : schacht.punkte) {
if(punkt.attr == PunktAttributAbwasser::DMP) {
hasDMP = true;
DMP = punkt;
} else if(punkt.attr == PunktAttributAbwasser::SMP) {
hasSMP = true;
SMP = punkt;
}
}
if(hasDMP and hasSMP) {
if(fuzzyCompare(abs(DMP.point.z-SMP.point.z), schacht.tiefe) == false) {
cout << "Redundante Information Schachttiefe vs DMP-SMP";
}
}
Diese Version hat folgende Nachteile und sleeping errors (Phew wo fang ich denn an).
a) WAS vs. WIE. In dem Code steht, WIE die zwei Punkte gefunden werden und danach WIE damit weiter verfahren wird. Aber es steht nicht da WAS überhaupt gemacht werden soll.
b) SMP und DMP sind ähnlich geschriebene Variablen die leicht verwechselt werden können. Das sind aber vorgegebene Abkürzungen die es einzuhalten gilt. Viel schlimmer aber sind die beiden zusätzlichen bool Variablen. Es können die bool und Punkt Variablen in jeder Kombination vertauscht werden.
c) Die Lebenszeit der DMP und SMP Variablen ist zu lange. Nach der letzten if() geht der Code normalerweise noch weiter. Hier könnte man eine der Variablen wieder benutzen, obwohl das gar nicht beabsichtigt ist. Ein ähnliches Problem hat die Sprache C mit der Laufvariable i in for() Schleifen. Das ist seit 1984 in C++ nie wieder ein Fehler gewesen.
d) RAII nicht beachtet. Erst wird der Punkt default initialisiert, und später vllt. einem anderen Punkt aus dem Schacht zugewiesen. Statt gleich die Zuweisung zu machen.
e) Und letztendlich wird eine Kopie des Punktes erzeugt. Das ist in diesem Fall aber das kleinere Übel. Referenzen, Indices, Iteratoren oder Pointer können alle invalid werden, sollte sich die Punkte in dem Schacht zwischenzeitlich ändern.
Hier sind Lösungsvorschläge:
a) Die Schleife muss in eine Funktion ausgelagert werden.
b) Punkt und bool Variable in ein neuen Datentyp koppeln.
c) Mehrere Möglichkeiten:
Scope durch zwei Klammern {} verkleinern.
Code in eine weitere Funktion packen.
C++17 "Selection statements with initializer" nutzen.
d) Auch (teil)gelöst durch Schleife in Subroutine.
Punkt a) ist trivial. Das sollte etwa so aussehen: auto DMP = punkt.DMP();
Punkt b) ist durch ein optional lösbar. Ich zeige Code mit std::optional und cpl::optional.
Punkt c) C++17 ist natürlich vorzuziehen.
Punkt d) Da cpl::optional default constructible brauchst ist hier RAII nicht so zu 100% enigehalten.
Fangen wir an:
cpl::optional<Punkt> Schacht::DMP() const {
for(const Punkt& punkt : punkte) {
if(punkt.attr == PunktAttributAbwasser::DMP) {
return punkt;
}
}
return {};
}
Im Erfolgsfall wird der Punkt zurück gegeben, sonst ein default cpl::optional, welches keine Daten hält. Für SMP() analog. Um zu testen ob ein std::optional ein Wert hält, gibt es die Funktionen has_value() oder operator bool. Ich möchte gern die Funktion beim Namen nennen, um den Code lesbarer zu halten. Aber punkt.has_value() ist.. seltsam. Jeder Punkt hat doch Werte? Daher nehme ich für cpl::optional lieber den Funktionsnamen exist(). Und so schauts aus:
auto DMP = schacht.DMP();
auto SMP = schacht.SMP();
if(DMP.exist() and SMP.exist()) {
if(fuzzyCompare(std::abs(DMP.point.z-SMP.point.z), schacht.tiefe) == false) {
cout << "Redundante Information Schachttiefe vs DMP-SMP";
}
}
Das ist schon wesentlich lesbarer. Hier empfiehlt sich die NUtzung von "auto", sonst müsste man umständlich cpl::optional schreiben. Also, Punkt a) b) d) erfüllt. Für c) nehme ich C++17, das sieht dann so aus:
if(auto DMP = schacht.DMP(); DMP.exist()) {
if(auto SMP = schacht.SMP(); SMP.exist()) {
if(fuzzyEqual(std::abs(DMP.point.z-SMP.point.z), schacht.tiefe) == false) {
std::cout << "Redundante Information Schachttiefe vs DMP-SMP";
}
}
}
Jetzt bin ich mir nur nicht sicher, ob die Initialisierung in der if() auch für mehrere Variablen funktioniert, oder ob der Compiler noch buggy ist. Aber jedenfalls, aus den anfänglichen 19 Zeilen Code sind es 7-8 geworden. Ein WIE wurde in ein WAS verwandelt. Der zweite Teil mit dem fuzzyEqual lass ich mal so stehen.
Hier die Implementierung von cpl::optional
namespace cpl {
template<class T>
struct optional : public T {
static_assert(is_default_constructible<T>::value);
optional() = default;
optional(const T& t)
: T(t), vorhanden(true)
{
}
explicit operator bool() const {
return vorhanden;
}
bool exist() const {
return vorhanden;
}
bool vorhanden = false;
};
}
Das nächstemal geht es mit std::optional weiter