C++ Review bzw. Feadback

[dominik_ (Gast)]

class A: public QObject
{
  Q_OBJECT
  
  public:
  EFILIBRARY_EXPORT A(bool boPower);
  EFILIBRARY_EXPORT ~A();
  .....
  signals:
  .....
  
  
  private:
  
  void *m_pvBclass;     // Bclass ist einen class 
  void *m_pvDclassPool; // DclassPool ist einen class
  void *m_pvEclassHold; //EclassHold ist einen class
  
  #define pmaGetBclass()      ((Bclass *) m_pvBclass) 
  #define pmaGetDclassPool() ((DclassPool *) m_pvDclassPool)  
  #define pmaGetEclassHold(a) (&((EclassHold*) m_pvEclassHold) [static_cast <int>(a)])
  
  ....
};

Mir geht darum vorallem um folgende:

...
private:
  
void *m_pvBclass;     // Bclass ist einen class 
void *m_pvDclassPool; // DclassPool ist einen class
void *m_pvEclassHold; //EclassHold ist einen class
  
#define pmaGetBclass()      ((Bclass *) m_pvBclass) 
#define pmaGetDclassPool() ((DclassPool *) m_pvDclassPool) 
#define pmaGetEclassHold(a) (&((EclassHold*) m_pvEclassHold) [static_cast <int>(a)])

Es ist echt scheisse implementiert.

[Bob (Gast)]

dominik_ schrieb:
> Es ist echt scheisse implementiert.

Da kann ich Dir nur voll zustimmen!

[dominik_ (Gast)]

und wie kann ich das besser implementieren?

[Sebastian V. (sebi_s)]

Statt void Pointer richtige Pointer auf die entsprechende Klasse. Dann 
die nicht mehr nötigen Makros loswerden.

[Oliver S. (oliverso)]

Wenns nicht irgendwo anders noch ein ganz abstrusen Trick verwendet 
wird, der die void Pointer braucht.

Wenn nicht, die member Variablen mit ihrem richtigen Typ anlegen, und 
die Setter/Getter als normale Zugriffsfunktionen im Header 
implementieren.

Oliver

[dominik_ (Gast)]

das hier:

  private:
  void *m_pvBclass;     // Bclass ist einen class 
  void *m_pvDclassPool; // DclassPool ist einen class
  void *m_pvEclassHold; //EclassHold ist einen class

kann es so umgeschrieben werden:

std::unique_ptr<Bclass> m_Bclass;
std::unique_ptr<DclassPool> m_DclassPool;
std::unique_ptr<EclassHold> m_EclassHold;

und die FunktionsMacros? Sorry

  #define pmaGetBclass()      ((Bclass *) m_pvBclass) 
  #define pmaGetDclassPool() ((DclassPool *) m_pvDclassPool)  
  #define pmaGetEclassHold(a) (&((EclassHold*) m_pvEclassHold) [static_cast <int>(a)])

[Daniel A. (daniel-a)]

Ich empfehle dir dich zuerst über Smartpointer zu informieren:
http://de.cppreference.com/w/cpp/memory/unique_ptr
http://de.cppreference.com/w/cpp/memory

Smartpointer sind dazu da die Lebenszeit der Objekte auf die sie Zeigen 
zu verwalten, und diese automatisch freizugeben wen sie nichtmehr 
verwendet werden.

Den Unique Pointer kannst du nur verwenden, wenn du willst, das nur 
dieser eine Pointer darauf existiert, und das Objekt automatisch 
freigegeben wird sobald der Unique Pointer nichtmehr existiert.

Da wir nicht wissen, wozu der Pointer weiterverwendet wird, können wir 
dir auch nicht sagen, ob du ihn hier verwenden kannst. Es ist aber 
relativ wahrscheinlich, dass du normale Pointer verwenden kannst:

Bclass* m_Bclass;
DclassPool* m_DclassPool;
EclassHold* m_EclassHold;

Zu den Funktionsmakros:

Bclass* pmaGetBclass(){
  return m_pvBclass;
}
 
DclassPool* pmaGetDclassPool(){
  return m_pvDclassPool;
}

EclassHold* pmaGetEclassHold( int a ){
  return m_pvEclassHold + a; // oder &pvEclassHold[a]
}

PS: Das war wohl nichts mit Stack-Overflow, XD
http://stackoverflow.com/questions/36093300/feedback-on-my-spaghetti-code
http://codereview.stackexchange.com/questions/123526/how-can-i-write-the-macro-method-better-please-feadback

[dominik_ (Gast)]

10000 danke

[dominik_ (Gast)]

Hallo,
noch eine kurze Frage, bei diese:

EclassHold* pmaGetEclassHold( int a ){
  return m_pvEclassHold + a; // oder &pvEclassHold[a]
}

bekomme ich diese Fehlermeldung:

 error C2036: 'EclassHold*' : unknown size

[Daniel A. (daniel-a)]

dominik_ schrieb:
> Hallo, noch eine kurze Frage, bei diese:
> bekomme ich diese Fehlermeldung:
> error C2036: 'EclassHold*' : unknown size

Grundlagen! Der Compiler weiss wohl schon, dass EclassHold eine klasse 
ist, aber nicht, was diese beinhaltet oder wie gross sie ist. Ersetze 
die Implementierungen mit Prototypen und packe die Implementierung in 
ein cpp file. Beim Cpp file bindest du alle klasse ein, die du benötigst 
(A und EclassHold)

// prototypen
Bclass* pmaGetBclass();
DclassPool* pmaGetDclassPool();
EclassHold* pmaGetEclassHold( int a );

// Cpp file
#include <path/to/a.hpp>
#include <path/to/EclassHold.hpp>

Bclass* A::pmaGetBclass(){
  return m_pvBclass;
}
 
DclassPool* A::pmaGetDclassPool(){
  return m_pvDclassPool;
}

EclassHold* A::pmaGetEclassHold( int a ){
  return m_pvEclassHold + a; // oder &pvEclassHold[a]
}

[dominik_ (Gast)]

Hallo Daniel,

danke es funktioniert aber es ist mit sehr grosse vorsicht zu geniessen.


Die Ressourcen:

Bclass* m_Bclass;
DclassPool* m_DclassPool;
DclassPool* m_EclassHold;

teilen mehrere Zeigern. Also es kommt dann shared_ptr zu verwendung.

Da ich Visual Studio 2008 benutze, kann ich nicht die Neuerungen in C11 
leider nicht davon gebarucht machen.
Ich habe die neue Boost1_60 Library runtergeladen und in Visual Studio 
eingebunden.

Wie soll denn die Zeiger aussehen?

-> Hier ist meinen Versuch:

  
  typedef boost::shared_ptr<Bclass>m_Bclass;
  typedef boost::shared_ptr<DclassPool>m_DclassPool;
  typedef boost::shared_ptr<DclassPool>m_EclassHold;

Ist das richtig so?

[Oliver S. (oliverso)]

Wie die Zeiger aussehen sollen, kommt darauf an, wie sie genutzt werden.

Oliver

[Daniel A. (daniel-a)]

dominik_ schrieb:
> Die Ressourcen:
...
> teilen mehrere Zeigern. Also es kommt dann shared_ptr zu verwendung.
>
> Da ich Visual Studio 2008 benutze, kann ich nicht die Neuerungen in C11
> leider nicht davon gebarucht machen.

C11 ist der falsche Standard. Du meinst C++11.


> Wie soll denn die Zeiger aussehen?
...
> Ist das richtig so?

Das typedef gehört da nicht hin, es sei denn du möchtest Datentypen 
statt Variablen definieren.

[dominik_ (Gast)]

@

das stimmt typedef gehört nicht hin:

 boost::shared_ptr<Bclass>m_Bclass;
 boost::shared_ptr<DclassPool>m_DclassPool;
 boost::shared_ptr<DclassPool>m_EclassHold;

[dominik_ (Gast)]

und wie sollen bitte die Funktionen implementiert werden?

[Daniel A. (daniel-a)]

Ich kenne mich mit boost nicht aus, vermutlich:

// Header file
boost::shared_ptr<Bclass> m_Bclass;
boost::shared_ptr<DclassPool> m_DclassPool;
boost::shared_array<DclassPool> m_EclassHold;

// cpp file

boost::shared_ptr<Bclass> A::pmaGetBclass(){
  return m_pvBclass;
}
 
boost::shared_ptr<DclassPool> A::pmaGetDclassPool(){
  return m_pvDclassPool;
}

Bclass& A::pmaGetEclassHold( int a ){
  return m_pvEclassHold[a];
}

[PittyJ (Gast)]

Braucht man boost da überhaupt? Geht das nicht ohne?
Letztens habe ich einen Vortrag gehört, deren Tenor war, dass boost 
durch Features von C++-14 komplett ersetzt werden könnte.
Was ist der Vorteil, wenn man da immer noch diese Extra-Bibliothek 
verwendet?

[Andre R. (physicist)]

PittyJ schrieb:
> Braucht man boost da überhaupt? Geht das nicht ohne?
> Letztens habe ich einen Vortrag gehört, deren Tenor war, dass boost
> durch Features von C++-14 komplett ersetzt werden könnte.
> Was ist der Vorteil, wenn man da immer noch diese Extra-Bibliothek
> verwendet?

boost ist eine Art (inoffizielle) Testbench für Dinge die zukünftig in 
den Standard fließen und wenn er einen aktuelleren Compiler benutzen 
würde, dann hättest Du recht, aber:

> Da ich Visual Studio 2008 benutze, kann ich nicht die Neuerungen in C11
> leider nicht davon gebarucht machen.

Das Ding ist leider Steinzeit.

Und natürlich gibt es laufend neues in boost.

Wobei ich auch die Frage stellen würde, ob ein Umstieg auf eine 
aktuellere Version nicht in Frage kommt, auch wenn dies ggf. mit 
Anpassungen verbunden sein kann.

[dominik_ (Gast)]

Daniel A. schrieb:
> Bclass& A::pmaGetEclassHold( int a ){
>   return m_pvEclassHold[a];
> }

ich bekomme folgende Fehlermeldung:

 error C2040: 'A::pmaGetEclassHold' : 'A&(int)' differs in levels of indirection from 'boost::shared_array<T> (int)'
2>        with
2>        [
2>            T=A
2>        ]

[Daniel A. (daniel-a)]

Ups, hatte beim Kopieren der Datentypen nicht aufgepasst:

// Header file
boost::shared_ptr<Bclass> m_Bclass;
boost::shared_ptr<DclassPool> m_DclassPool;
boost::shared_array<EclassHold> m_EclassHold;

// cpp file

boost::shared_ptr<Bclass> A::pmaGetBclass(){
  return m_pvBclass;
}
 
boost::shared_ptr<DclassPool> A::pmaGetDclassPool(){
  return m_pvDclassPool;
}

EclassHold& A::pmaGetEclassHold( int a ){
  return m_pvEclassHold[a];
}

[dominik_ (Gast)]

// Header file
boost::shared_ptr<Bclass> m_Bclass;
boost::shared_ptr<DclassPool> m_DclassPool;
boost::shared_array<EclassHold> m_EclassHold;

boost::shared_ptr<Bclass>pmaGetBclass();
boost::shared_ptr<DclassPool>pmaGetDclassPool();
EclassHold& A::pmaGetEclassHold( int a );

boost::shared_ptr<Bclass> A::pmaGetBclass(){
  return m_pvBclass;
}
 
boost::shared_ptr<DclassPool> A::pmaGetDclassPool(){
  return m_pvDclassPool;
}

EclassHold& A::pmaGetEclassHold( int a ){
  return m_pvEclassHold[a];
}

So passt danke

[dominik_ (Gast)]

Noch eine Frage bitte:

ich habe sehr viele funktionen, die so in der Form sind:

static void fkt_1(Bclass* bClass_p, DclassPool* dClass_p);
static void fkt_2(Bclass* bClass_p, DclassPool* dClass_p, EclassHold*eClassHold);
....

Das heisst alle Funktionen müssen umgeschrieben werden so ungefähr:

static void fkt_1(boost::shared_ptr<Bclass> bClass_p, boost::shared_ptr<DclassPool> dClass_p);
...

Ist das richtig?

Sorry bitte

[Daniel A. (daniel-a)]

Ja

[Torsten R. (Firma: Torrox.de) (torstenrobitzki)]

dominik_ schrieb:
> Noch eine Frage bitte:
>
> ich habe sehr viele funktionen, die so in der Form sind:
>

> static void fkt_1(Bclass* bClass_p, DclassPool* dClass_p);
> static void fkt_2(Bclass* bClass_p, DclassPool* dClass_p, 
> EclassHold*eClassHold);
> ....
>

>
>
> Das heisst alle Funktionen müssen umgeschrieben werden so ungefähr:
>

> static void fkt_1(boost::shared_ptr<Bclass> bClass_p, 
> boost::shared_ptr<DclassPool> dClass_p);
> ...
>

>

Wahrscheinlich eher nein. Wenn die Funktion eine zusätzliche Referenz 
(nicht im C++ Sprachsinn) auf das übergebene Objekt speichern möchte, 
dann braucht die Funktion den shared pointer. Aber auch dann spricht 
wenig dagegen, die shared pointers per const Referenz zu übergeben:

    static void fkt_1(
        const boost::shared_ptr<Bclass>& bClass_p, 
        const boost::shared_ptr<DclassPool>& dClass_p);
...

Wenn die Funktion nur eine Referenz auf ein Objekt braucht, und diese 
Referenz nicht optional ist (die Funktion nicht erwartet, dass der 
Pointer 0 sein kann), dann solltest Du es durch eine C++ Referenz 
ersetzen:

    static void fkt_1(
        Bclass& bClass_p, 
        DclassPool& dClass_p);
...

Im Idealfall nimmst Du Deine Umbauarbeiten zum Anlass, noch mal das 
Thema const correctness in Deinem C++ Buch nachzuschlagen und dass ggf. 
auch gerade zu ziehen.

mfg Torsten