Handle-Kapselung: Methoden const?

Hi!
Mich interessiert, welchen Stil ihr besser findet.
Angenommen, man möchte ein WinAPI void* handle oder ein unsigned int socket kapseln. Sollten dann so Funktionen wie Handle::Close() { CloseHandle(handle) } oder Socket::Shutdown() { shutdown(socket, SD_BOTH); } const sein, also für konstante Instanzen aufgerufen werden können? Immerhin wird ja nicht die Klasse selbst verändert.

SeppJ

non-const.

Constness bedeutet nicht Byteidentität der direkten Member der Klasse, sondern Identität bezüglich des logischen Inhalts^*. Und zum logischen Inhalt dieser Klasse gehört der handle/socket - und somit auch dessen Zustand - eindeutig dazu. Dafür ist der Wrapper schließlich da!

Sonst könnte man auch argumentieren, dass Zugriffsfunktionen von Containerklassen const sein sollten, da über sie Objekte verändert werden, die im Container gespeichert sind, aber nicht der Container selber. Ich denke, diese Argumentation würdest du auch als absurd ansehen, oder?

^*: Man kann es auch so formulieren: Wann ist Operation A const? Wenn sämtliche anderen Operationen, die auf dem Objekt möglich sind, vor und nach dem Ausführen von Operation A in jedem(!) Fall das gleiche Ergebnis haben, dann ist A eine konstante Methode. Das wäre hier nicht der Fall, da vor dem Shutdown eine typische Zugriffsoperation erfolgreich wäre, nach dem Shutdown aber scheitern muss. Daher kann der Shutdown nicht const sein.

hustbaer

@loboo
Eine Methode wie Handle::Close() { CloseHandle(handle) } sollte es IMO gar nicht geben.
Weil sie das "Smart Handle" in einem ungültigen Zustand zurücklässt (handle != INVALID_HANDLE_VALUE, aber schon geschlossen).

Ich mach das immer mit Reset Methoden ala

void Handle::Reset()
{
    HANDLE old = handle;
    handle = INVALID_HANDLE_VALUE;
    if (old != INVALID_HANDLE_VALUE)
        ::CloseHandle(old);
}

Die ist dann natürlich nicht const, da das Objekt dabei ja selbst verändert wird.

Ansonsten sind Handles konzeptuell Zeiger, und so behandle ich sie auch: eine WriteFile Methode einer FileHandle Klasse ist const .
Genau so wie ich mit einem FILE* const die Funktion fwrite aufrufen kann, oder mit einem std::shared_ptr<std::fstream> const die Funktion write .

Wobei ich solche Funktionen genaugenommen gar nicht implementiere. Nicht in der SmartHandle Klasse, dafür ist die nicht zuständig. Die Aufgabe von SmartHandle ist für mich RAII - nicht das Kapseln der ganzen API Calls.

Das Kapseln der API Calls macht dann eine andere Klasse, sagen wir File , die ein SmartHandle als Member hat.
Und in der File Klasse trifft dann SeppJ's Argumentation für mich zu. In der SmartHandle Klasse dagegen nicht.

Stimmt, danke!

Aha, hmm

SeppJ

hustbaer schrieb:

Das Kapseln der API Calls macht dann eine andere Klasse, sagen wir File , die ein SmartHandle als Member hat.
Und in der File Klasse trifft dann SeppJ's Argumentation für mich zu. In der SmartHandle Klasse dagegen nicht.

Ja, hier hätte man klären müssen, was die Klasse überhaupt genau macht. Wenn sie wirklich nur den Zeiger hält, dann passt deine Erklärung. Ich bin nach dem, was ich gesehen habe, davon ausgegangen, dass das Ding die ganze API reimplementiert. Denn nur dann macht ein Close Sinn, ansonsten bräuchte man ein Reset, wie du es beschreibst.

Nur mal so nebenbei, warum nicht

void Handle::Close()
{
    if(handle != INVALID_HANDLE_VALUE)
    {
        CloseHandle(handle);
        handle = INVALID_HANDLE_VALUE;
    }
}

? Wozu old? Und Close oder Reset? Meint ihr, nur wegen der Bedeutung des Namens?

hustbaer

Reset, weil man üblicherweise noch nen Overload macht der nen HANDLE Parameter nimmt. Der gibt dann das alte Handle frei und setzt das neue rein. (Bzw. man kann natürlich auch nen Default-Wert für den Parameter machen.)
Und dann ist Reset ("re set") eben passender als Close

Bzw. auch einfach: weil das bei std::shared_ptr und std::unique_ptr auch so heisst

Und deine Variante ist in diesem Fall auch OK, da die WinAPI CloseHandle Funktion keine Exception werfen kann. Im allgemeinen Fall ist meine IMO besser, da dann auch im Fall wenn die "Close" Funktion eine Exception wirft (was sie nicht tun sollte, aber manche tun es halt trotzdem), das Objekt danach "leer" ist. Und damit ein zweiter (dritter, vierter, ...) Aufruf von "Close" auf das selbe Handle verhindert wird.

Wie gesagt in diesem Fall unnötig, aber wenn man es sich mal angewöhnt hat...

Wobei... die noch "schönere" Variante wäre vermutlich copy & swap zu verwenden.

Also

class Handle : private noncopyable
{
public:
    explicit Handle(HANDLE handle = INVALID_HANDLE_VALUE)
        : m_handle(handle)
    {
    }

    ~Handle()
    {
        if (IsValid())
            CloseHandle(m_handle);
    }

    HANDLE Get() const
    {
        return m_handle;
    }

    bool IsValid() const
    {
        return m_handle != INVALID_HANDLE_VALUE;
    }

    void Reset(HANDLE handle = INVALID_HANDLE_VALUE)
    {
        assert(handle != m_handle || handle == INVALID_HANDLE_VALUE);
        Handle(handle).Swap(*this);
    }

    void Swap(Handle& other)
    {
        std::swap(m_handle, other.m_handle);
    }
};

Noch besser natürlich wenn man noch Move-Semantik einbaut, aber ich bin grad zu faul das runterzutippen

ps: Sinnvollerweise implementiert man das auch nur 1x, und packt die "Close" und "IsValid" Funktionen, sowie den INVALID_HANDLE_VALUE Wert in eine Traits-Klasse. Und HANDLE wird zum Template-Parameter.
Dann muss man bloss noch zwei triviale Funktionen implementieren und Typ und "Invalid" Wert angeben, und hat ne passende HANDLE Klasse.
Also CloseHandle/h != INVALID_HANDLE_VALUE && h != 0/HANDLE/INVALID_HANDLE_VALUE für KERNEL32 Handles, DeleteObject/h != 0/HGDIOBJ/0 für GDI Handles usw.

EDIT: Fehler korrigiert (siehe Beitrag von TyRoXx)

TyRoXx

hustbaer schrieb:

void Reset(HANDLE handle = INVALID_HANDLE_VALUE)
    {
        if (handle != m_handle)
            Handle(handle).Swap(*this);
    }

Gibt es einen guten Grund dafür, dass da nicht assert(handle != m_handle) steht?

hustbaer

Nein, gibt es nicht.

Es macht natürlich keinen Sinn das zu erlauben.
Die implizite Precondition für Reset muss ja sein dass der Aufrufer "ownership" des Handles hat. Denn diese wird ja mit Reset transferiert.
Wenn aber handle == m_handle, dann hat ja *this schon "ownership", und daher kann der Aufrufer auch nicht "ownership" haben.

Ich hab' den Code schnell für's Forum runtergetippt, und mir dann auf einmal eingebildet da müsste ein if hin - "self assignment" und so. Nur dass wir hier ja gar keine Assignment-Semantics haben. *patsch*

Danke für den Hinweis! Hab's korrigiert.

ps: es muss natürlich assert(handle != m_handle || handle == INVALID_HANDLE_VALUE); heissen