Schlechter Programmierstil

Hallo,

meine Frage ist, ob es schlechter Programmierstil ist, einen Vergleichsoperator für eine Klasse zu überladen, der als Parameter nicht ein Objekt der Klasse erhält.
Ich gebe mal ein Beispiel:

// auto.h
#include <string>

class AUTO {
private:
    std::string kennzeichen;
    std::string marke;
    std::string produkt;

public:
    AUTO(const std::string _kennzeichen,
         const std::string _marke,
         const std::string _produkt);

    // Achtung - der Operator der kein AUTO als Parameter erwartet
    bool operator ==(const std::string _kennzeichen) const;
};

// auto.cpp
#include "auto.h"

AUTO::AUTO(const std::string _kennzeichen,
           const std::string _marke,
           const std::string _produkt) : kennzeichen(_kennzeichen),
                                         marke(_marke),
                                         produkt(_produkt) {}

bool AUTO::operator ==(const std::string _kennzeichen) const {
    return kennzeichen == _kennzeichen;
}

Der ==-Operator vergleicht hier also die Kennzeichen und gibt true zurück, wenn die Kennzeichen übereinstimmen.

a) ist das schlechter Programmierstil?
b) ist es schlechter Programmierstil später in einer Liste von AUTO-Objekten sowas zu machen: std::find(auto_list.begin(), auto_list.end(), "ABC-123456");

theta

Ich denke es ist irgendwie unnatürlich.
Ich würde eher Funktionen dazu schreiben.

Für das suchen in der Liste kannst Du ja ein einfacher Funktor schreiben.

JustAnotherNoob

Identifier sollten nicht mit _ beginnen, diese Namen sind für den Compiler reserviert.
Außerdem würde ich ja sagen, mach lieber ein bool hat_kennzeichen(std::string const& kennzeichen) const;
Wobei es nicht grundsätzlich so ist, eine Komplexe Zahl mit einem Integer zu vergleichen wäre eher natürlich, weil beides eine Zahl ist, aber ein Auto hat ein Kennzeichen und hat nichts mit ihm gemeinsam.

Bashar

JustAnotherNoob schrieb:

Identifier sollten nicht mit _ beginnen, diese Namen sind für den Compiler reserviert.

Das stimmt so pauschal nur, wenn nach dem _ ein Großbuchstabe oder ein zweiter _ kommt. Namen mit _kleinbuchstabe sind nur reserviert, wenn sie external linkage haben.

pumuckl

Faustregel für Vergleichsoperatoren: Sie sind nur sinnvoll wenn sie ineinander umwandelbar sind (z.B. verschiedene Zahlentypen, abgeleitete Klassen etc.). Ein Auto ist aber kein Kennzeichen, genauso ist ein Kennzeichen kein Auto. Ergo kann ein Kennzeichen nicht gleich einem Auto sein.
Ich würde eine von den zwei folgenden Varianten nehmen:
- eine Methode bool hatKennzeichen(std::string const& kennzeichen)
oder, wenn das Kennzeichen eh des öfteren gelesen werden muss
- eine Methode std::sting const& getKennzeichen() , die das Kennzeichen zurückgibt (und dann einen normalen String-Vergleich)

/edit:
zu den Unterstichen:

17.4.3.1.2 Global names [lib.global.names]
1 Certain sets of names and function signatures are always reserved to the implementation:

Each name that contains a double underscore (_ _) or begins with an underscore followed by an upper case letter (2.11) is reserved to the implementation for any use.
Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace. Such names are also reserved in namespace ::std

ja, Unterstriche mit folgenden Kleinbuchstaben kann man unter bestimmten Umständen verwenden. Man kann sich natürlich diese Ausnahmen von der Regel merken und führende Unterstriche benutzen - und bei Gelegenheit auf die Nase fallen wenn man die Ausnahme doch nicht ganz richtig im Kopf hatte. Einfacher ists, gänzlich auf führende Unterstriche zu verzichten, das beschränkt einen nicht merklich in der Wahl der Variablennamen. Imho iriitieren sie eh nur und schränken die Lesbarkeit ein, das Auge stolpert einfach drüber beim Lesen.

Einen Punkt habe ich noch, den ich auch als zumindest ungewöhnlichen Stil bewerte: Bezeichner die ausschließlich in Großbuchstaben geschrieben werden, sind in den meisten Coding-Standards für Makros und evtl. statische Konstanten und/oder enums reserviert. Der Klassenname AUTO verstößt gegen diese inoffizielle Konvention und ist daher für viele auch ein Stolperstein beim Lesen.

Nexus

Zudem bringt

const std::string _kennzeichen

nicht sehr viel. Erstens wird eine unnötigte Kopie angelegt (von Optimierungen, auf die man sich nicht verlassen kann, abgesehen). Zweitens kann es dem Aufrufer eigentlich relativ egal sein, ob die Kopie innerhalb der Funktion verändert wird, von daher finde ich const -Parameter nicht wirklich sinnvoll.

Was eine Kopie vermeidet und auch eher eingesetzt wird, ist eine Referenz auf einen const std::string :

const std::string& kennzeichen // oder
std::string const& kennzeichen

Hallo,

danke euch für die Antworten.
Zu der Idee mit dem Funktor habe ich jetzt noch mal eine Frage - auch Programmierstil bedingt.

Schreibt man für verschiedene Eigenschaften verschiedene Funktoren, oder kann man einen verwenden, der eben noch einen Wert übergeben bekommt, der angibt welche Daten verglichen werden sollen?

volkard

daersc schrieb:

Schreibt man für verschiedene Eigenschaften verschiedene Funktoren

ja.