C++ Klasse - stil und fehler

[Braunstein]

im Header ja. In der cpp kannst du ja das using namespace wieder verwenden.
Die set- und get Funktionen mit nur einer Zeile, kannst du auch gleich im Header mit definieren (dann werden sie auch gleich inline).

[blan]

Braunstein schrieb:

im Header ja. In der cpp kannst du ja das using namespace wieder verwenden.
Die set- und get Funktionen mit nur einer Zeile, kannst du auch gleich im Header mit definieren (dann werden sie auch gleich inline).

hab ich mir auch überlegt aber dachte es könnte dann vll unübersichtlich werden und wollte die ganze implementation dann in die cpp-datei reinschreiben.

sollte ich eigentlich als extension von headern *.hpp benutzten?

mfg blan

[MFK]

- Was passiert bei mehreren C_Shout-Objekten (init/shutdown)?
- Const-correctness (z.b: für Getter)
- get_version static?

[Konrad Rudolph]

- Wieso ist alles virtuell?
- Benennungskonvention ("C_Shout"? Was spricht gegen "Shout"?)
- Redundante Funktionsaufrufe ('string get_version()' und 'void get_version(int, int, int)') -- puffer intern in der Klasse den Aufruf (ich nehme nicht an, dass sich die Version während der Laufzeit ändert?)
- Wozu brauchst Du im Header <iostream>? Falls Du irgendwo noch Streams einsetzt, sollte trotzdem <iosfwd> reichen (enthält Vorwärtsdeklarationen für die <iostream>-Typen).

[blan]

okay, aus sicherheitsgründen sollte ich wohl wirklich init und shutdown nicht in construktor / deconstructor bauen oder?

hab mir ein paar andere Quellcodes angeschaut und da ham die ihre Klasse so genannt, aber Shout ist kein problem.

das mit der version ist auch eine gute idee..

was stimmt mit den "const" net?

danke!

mfg blan

[THX 1138]

blan schrieb:

okay, aus sicherheitsgründen sollte ich wohl wirklich init und shutdown nicht in construktor / deconstructor bauen oder?

Kannst Du schon. Wenn Du mehrere Objekte hast, hilft Dir vielleicht ein Reference Counter.

[LordJaxom]

virtual const string get_user(void);

Wozu ne konstante Kopie zurückgeben? Ändern interessiert Deine Klasse eh nicht. Und das Shout-Objekt wird auch nicht geändert, im Getter.

virtual const string& get_user() const;

Oder, wenn Du eine Kopie zurückgeben magst (warum auch immer), dann die wenigstens nicht konstant

virtual string get_user() const;

EDIT: (void) auch noch entschlackt

[blan]

okay, ich hab jetzt mal alles so umgesetzt wie ichs verstanden habe, wenn ihr noch was findet sagt bescheid.

#ifndef _SHOUT_H_
#define _SHOUT_H_

#include <iostream>
#include <string>
#include <shout/shout.h>

class Shout
{
  private:
    shout_t *m_shout;
	static unsigned short m_count;

  protected:

  public:
     Shout();
    ~Shout();

    virtual int open();
    virtual int close();

    static void get_version(int &major,
                             int &minor,
                             int &patch);

    static std::string get_version();

    virtual int set_host(const std::string &host);
    virtual std::string get_host();

    virtual int set_port(const unsigned short &port);
    virtual unsigned short get_port();

    virtual int set_user(const std::string &user);
    virtual std::string get_user();

    virtual int set_password(const std::string &password);
    virtual std::string get_password();

    virtual int set_mount(const std::string &mount);
    virtual std::string get_mount();
};

#endif /* _SHOUT_H_ */

#include "shout.h"

unsigned short Shout::m_count = 0;

Shout::Shout()
{
  if(m_count == 0)
    shout_init();

  m_count++;

  m_shout = shout_new();
}

Shout::~Shout()
{
  m_count--;

  if(m_count == 0)
    shout_shutdown();

  shout_free(m_shout);
}

int Shout::open()
{
  return shout_open(m_shout);
}

int Shout::close()
{
  return shout_close(m_shout);
}

void Shout::get_version(int &major, int &minor, int &patch)
{
  shout_version(&major, &minor, &patch);
}

std::string Shout::get_version()
{
  return shout_version(0, 0, 0);
}

int Shout::set_host(const std::string &host)
{
  return shout_set_host(m_shout, host.c_str());
}

std::string Shout::get_host()
{
  return shout_get_host(m_shout);
}

int Shout::set_port(const unsigned short &port)
{
  return shout_set_port(m_shout, port);
}

unsigned short Shout::get_port()
{
  return shout_get_port(m_shout);
}

int Shout::set_user(const std::string &user)
{
  return shout_set_user(m_shout, user.c_str());
}

std::string Shout::get_user()
{
  return shout_get_user(m_shout);
}

int Shout::set_password(const std::string &password)
{
  return shout_set_password(m_shout, password.c_str());
}

std::string Shout::get_password()
{
  return shout_get_password(m_shout);
}

int Shout::set_mount(const std::string &mount)
{
  return shout_set_mount(m_shout, mount.c_str());
}

std::string Shout::get_mount()
{
  return shout_get_mount(m_shout);
}

mfg blan

[.filmor]

blan schrieb:

sollte ich eigentlich als extension von headern *.hpp benutzten?

IMHO ja. Das stellt nochmal eindeutig klar, dass dies ein C++- und kein C-Header ist (der "echte" libshout-Header scheint ja auch shout.h zu heißen). Die Alternativen, die ich kenne sind nicht so doll (.H macht unter Windows Probleme, ein doppeltes H ist in Deutschland negativ besetzt).

Namen die mit Unterstrich anfangen gehören auch dem Compiler. Mach einfach SHOUT_HPP.

Ich würde bezüglich der Zugriffsbereiche die Ordnung public, private vorziehen (protected benutzt du eh nicht!), da der Anwender nur das Interface braucht und es deshalb direkt sehen sollte. Außerdem würde ich an deiner Stelle die Methoden noch passend kommentieren.

Desweiteren sind alle getter als const zu kennzeichnen und sollten (für Nicht-Integrale Typen) eine const Referenz zurückliefern.

Einzeiler sollten entweder als inline gekennzeichnet werden oder direkt im Header stehen.

Wenn dein Wrapper mehr als diese eine Klasse enthält gehört er in einen eigenen Namensraum.

Du hast immernoch keine Antwort darauf geliefert, warum alle Methoden virtuell ist und (falls dies nötig sein sollte) hast noch immer keinen virtuellen Destruktor.

Frag nach, wenn du etwas nicht verstehst, aber übergeh bitte nicht einfach Hinweise.

[blan]

okay, aber da die klasse noch nicht fertig ist weisst du garnicht ob ich protected benutzen werde oder nicht ich habe die einfach alle mal virtuell gemacht - werd ich bei bedarf noch abändern. wenn ihr mir des mit dem const nochmal erklären könntet wär ich sehr dankbar bzw. nen tutorial wo des beschrieben ist. was macht zB ein const am anfang anders als am ende?

mfg blan

[.filmor]

Das const am Anfang gehört zum Rückgabetyp (der dann statt std::string z.B. const std::string& wird), während das const am Ende der Deklaration dafür sorgt, dass diese Methode erstens nur lesend auf das Objekt, über das sie aufgerufen wurde, zugreifen und zweitens kann sie deshalb auch auf const-Objekten aufgerufen werden. getter brauchen grundsätzlich keinen Schreibzugriff, deshalb sind diese Methoden immer const.

Übrigens macht man sich eigentlich bevor man die Klasse hinschreibt über deren Struktur und Verwendung Gedanken

[akari]

blan schrieb:

wenn ihr mir des mit dem const nochmal erklären könntet wär ich sehr dankbar bzw. nen tutorial wo des beschrieben ist.

Hier nochmal eine komplette Zusammenfassung der Const-Correctness

bis bald
akari

[blan]

okay, das mit dem const am ende versteh ich jetzt - die method kann dann keine klassen-variable verändern. aber was macht ein const am anfang einer methode, zB

const string getValue();

wzt soll denn das const bewirken - hab die situation in keinem tutorial gefunden?

mfg blan

[THX 1138]

blan schrieb:

okay, das mit dem const am ende versteh ich jetzt - die method kann dann keine klassen-variable verändern. aber was macht ein const am anfang einer methode, zB

const string getValue();

wzt soll denn das const bewirken - hab die situation in keinem tutorial gefunden?

mfg blan

Wie .filmor sagte, das gehört zum Rückgabewert.

[LordJaxom]

blan schrieb:

const string getValue();

wzt soll denn das const bewirken - hab die situation in keinem tutorial gefunden?

Wundert mich nicht - weil's wenig Sinn ergibt. Anders sieht das bei einer konstanten Referenz aus:

const string& getValue();

Vorteil Referenz: Es wird direkt auf den String in Deinem Objekt zugegriffen, ohne die Notwendigkeit eine Kopie zu erstellen (was oben passieren würde).
Vorteil const: Niemand kann den String aus Deinem Objekt ändern. Ohne const wäre im zweiten Fall nämlich folgendes möglich:

objekt.getValue() = "hallo, welt";

[blan]

okay, so ist das logisch - aber wenn ich folgendes hab

const string &getValue()
{
  string y = "hello world";
  return y;
}

string a = getValue();

dann existiert y doch garnicht mehr und es kommt zu einem fehler oder?

mfg blan

[LordJaxom]

Wenn Du einen guten Compiler hast kommt es zu einer Warnung a la "returning reference to temporary" - was allerdings die Compilierung nicht verhindert. Das Laufen zu lassen ist undefiniert.

[blan]

jo hab ich bemerkt, mein compiler hat eine warnung ausgegeben.

also seh ich das richtigt, die referenz bei einer methode bringt nur was um den wert einer variable die dauerhaft exisiert zurückzugeben?

mfg blan

[akari]

Hallo

also seh ich das richtigt, die referenz bei einer methode bringt nur was um den wert einer variable die dauerhaft exisiert zurückzugeben?

Ja du solltest unbedingt vermeiden, temporäre Werte per Referenz oder Pointer zu übergeben.

bis bald
akari

[blan]

also ich bin wieder ein stück weiter und atm sieht die klasse wie folgt aus (das streamen funktioniert schon)

#ifndef _SHOUT_H_
#define _SHOUT_H_

#include <iostream>
#include <string>
#include <shout/shout.h>

class Shout
{
  private:
    shout_t *m_shout;
    static unsigned short m_count;

  protected:

  public:
     Shout();
    ~Shout();

    static void get_version(int &major,
                             int &minor,
                             int &patch);

    static std::string get_version();

    std::string get_error() const;
    int get_errno() const;

    bool get_connected() const;

    int open();
    int close();

    int set_host(const std::string &host);
    std::string get_host() const;

    int set_port(const unsigned short &port);
    unsigned short get_port() const;

    int set_user(const std::string &user);
    std::string get_user() const;

    int set_password(const std::string &password);
    std::string get_password() const;

    int set_mount(const std::string &mount);
    std::string get_mount() const;

    int set_protocol(const unsigned int &portocol);
    unsigned int get_protocol() const;

	int set_format(const unsigned int &format);
	unsigned int get_format() const;

    int set_name(const std::string &name);
    std::string get_name() const;

    int set_url(const std::string &url);
    std::string get_url() const;

    int set_genre(const std::string &genre);
    std::string get_genre() const;

	int set_public(const unsigned int &make_public);
	unsigned int get_public() const;

    int set_agent(const std::string &agent);
    std::string get_agent() const;

    int set_description(const std::string &description);
    std::string get_description() const;

    int set_dumpfile(const std::string &dumpfile);
    std::string get_dumpfile() const;

    int set_audio_info(const std::string &name,
                       const std::string &value);

    std::string get_audio_info(const std::string &name);

	int set_nonblocking(const unsigned int &nonblocking);
	unsigned int get_nonblocking() const;

	int send(const unsigned char *data,
             const size_t &len);

	ssize_t send_raw(const unsigned char *data,
                     const size_t &len);

	void sync();
};

#endif /* _SHOUT_H_ */

#include "shout.h"

unsigned short Shout::m_count = 0;

Shout::Shout()
{
  if(m_count == 0)
    shout_init();

  m_count++;

  m_shout = shout_new();
}

Shout::~Shout()
{
  m_count--;

  if(m_count == 0)
    shout_shutdown();

  shout_free(m_shout);
}

void Shout::get_version(int &major,
                        int &minor,
                        int &patch)
{
  shout_version(&major, &minor, &patch);
}

std::string Shout::get_version()
{
  return shout_version(0, 0, 0);
}

std::string Shout::get_error() const
{
  return shout_get_error(m_shout);
}

int Shout::get_errno() const 
{
  return shout_get_errno(m_shout);
}

bool Shout::get_connected() const
{
  if(shout_get_connected(m_shout) == SHOUTERR_CONNECTED)
    return true;
  else
    return false;
}

int Shout::open()
{
  return shout_open(m_shout);
}

int Shout::close()
{
  return shout_close(m_shout);
}

int Shout::set_host(const std::string &host)
{
  return shout_set_host(m_shout, host.c_str());
}

std::string Shout::get_host() const
{
  return shout_get_host(m_shout);
}

int Shout::set_port(const unsigned short &port)
{
  return shout_set_port(m_shout, port);
}

unsigned short Shout::get_port() const
{
  return shout_get_port(m_shout);
}

int Shout::set_user(const std::string &user)
{
  return shout_set_user(m_shout, user.c_str());
}

std::string Shout::get_user() const
{
  return shout_get_user(m_shout);
}

int Shout::set_password(const std::string &password)
{
  return shout_set_password(m_shout, password.c_str());
}

std::string Shout::get_password() const
{
  return shout_get_password(m_shout);
}

int Shout::set_mount(const std::string &mount)
{
  return shout_set_mount(m_shout, mount.c_str());
}

std::string Shout::get_mount() const
{
  return shout_get_mount(m_shout);
}

int Shout::set_protocol(const unsigned int &protocol)
{
  return shout_set_protocol(m_shout, protocol);
}

unsigned int Shout::get_protocol() const
{
  return shout_get_protocol(m_shout);
}

int Shout::set_name(const std::string &name)
{
  return shout_set_name(m_shout, name.c_str());
}

int Shout::set_format(const unsigned int &format)
{
  return shout_set_format(m_shout, format);
}

unsigned int Shout::get_format() const
{
  return shout_get_format(m_shout);
}

std::string Shout::get_name() const
{
  return shout_get_name(m_shout);
}

int Shout::set_url(const std::string &url)
{
  return shout_set_url(m_shout, url.c_str());
}

std::string Shout::get_url() const
{
  return shout_get_url(m_shout);
}

int Shout::set_genre(const std::string &genre)
{
  return shout_set_genre(m_shout, genre.c_str());
}

std::string Shout::get_genre() const
{
  return shout_get_genre(m_shout);
}

int Shout::set_public(const unsigned int &make_public)
{
  return shout_set_public(m_shout, make_public);
}

unsigned int Shout::get_public() const
{
  return shout_get_public(m_shout);
}

int Shout::set_agent(const std::string &agent)
{
  return shout_set_agent(m_shout, agent.c_str());
}

std::string Shout::get_agent() const
{
  return shout_get_agent(m_shout);
}

int Shout::set_description(const std::string &description)
{
  return shout_set_description(m_shout, description.c_str());
}

std::string Shout::get_description() const
{
  return shout_get_description(m_shout);
}

int Shout::set_dumpfile(const std::string &dumpfile)
{
  return shout_set_dumpfile(m_shout, dumpfile.c_str());
}

std::string Shout::get_dumpfile() const
{
  return shout_get_dumpfile(m_shout);
}

int Shout::set_audio_info(const std::string &name,
                          const std::string &value)
{
  return shout_set_audio_info(m_shout, name.c_str(), value.c_str());
}

std::string Shout::get_audio_info(const std::string &name)
{
  return shout_get_audio_info(m_shout, name.c_str());
}

int Shout::set_nonblocking(const unsigned int &nonblocking)
{
  return shout_set_nonblocking(m_shout, nonblocking);
}

unsigned int Shout::get_nonblocking() const
{
  return shout_get_nonblocking(m_shout);
}

int Shout::send(const unsigned char *data,
                const size_t &len)
{
  return shout_send(m_shout, data, len);
}

ssize_t Shout::send_raw(const unsigned char *data,
                        const size_t &len)
{
  return shout_send_raw(m_shout, data, len);
}

void Shout::sync()
{
  shout_sync(m_shout);
}

(falles es jemand intressiert :D)

kleine frage hätt ich da noch:

1. kann mit man c++ streams auch pipes öffnen und wann ja wie?

2. wie schreibt man sowas in c++... ich hab mit www.cppreference.com und ähnliches angeschaut aber der stream lief so nie

unsigned char buffer[4096];
	  size_t read;
	  FILE *hFile;

		hFile = fopen("05-schritt_fuer_schritt_feat._gnom-blz.ogg", "r");

		  while(1)
		  {
			read = fread(buffer, 1, sizeof(buffer), hFile);

			if(read > 0)
			{
			  if(shout.send(buffer, read) != SHOUTERR_SUCCESS)
			  {
//				printf("[DEBUG] Send error: %s (%d)", shout.get_error(), shout.get_errno());
				break;
			  }
			}
			else
			{
			  break;
			}

			shout.sync();
		  }

		fclose(hFile);

mfg blan