cpp code Spagetti

--> Bitte um eine Feadback

Folgende c++ Code habe ich:

class A: public Object
{
	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) // soll ich so 
	#define pmaGetEclassHold(a) (&((EclassHold*) m_pvEclassHold) [static_cast <int>(a)])

	....
};

Meine Frage bezieht sich genau auf diese:

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) // soll ich so 
	#define pmaGetEclassHold(a) (&((EclassHold*) m_pvEclassHold) [static_cast <int>(a)])

Danke

Feedback zu void* und Funktionsmakros?
Wer so etwas schreibt, gehört geschlagen.

Und wer HN benutzt wird auch noch geteert und gefedert

Wenn ich euch sage, habe ich so was nicht geschrieben und ich verstehe auch nicht was der Typ damit bezwecken möchte.

Man sollte nie einen void* benutzen. Das weiss ich schon

Wie kann ich aber das besser in cpp implementieren?
Ich entschuldige mich auch für dieses Spagetti cpp Code

Ist_Das_CPP schrieb:

Wenn ich euch sage, habe ich so was nicht geschrieben und ich verstehe auch nicht was der Typ damit bezwecken möchte.
[...]
Wie kann ich aber das besser in cpp implementieren?

Mechanics

Irgendwie werd ich das Gefühl nicht los, dass du das doch geschrieben hast...

Was willst du damit erreichen?

Ich kann es dir leider auch nicht helfen aber egal was das geschrieben hat, würde mich auch interessieren wie es besser zu implementieren ist.

Solche Code ist vielleicht schlecht, hat aber mit Spaghetti-Code überhaupt nichts zu tun.

Kenner der Spaghetti schrieb:

Solche Code ist vielleicht schlecht, hat aber mit Spaghetti-Code überhaupt nichts zu tun.

Ja aber man hätte das bestimmt besser implementieren können.

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

--> Warum ist das nicht gut?

bzw.

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

manni66 schrieb:

Feedback zu void* und Funktionsmakros?
Wer so etwas schreibt, gehört geschlagen.

ja danke das hatte er und ich auch bestimmt verdient aber um mich zu verbessern brauche ich eure Feadback
Danke in voraus

Ist_Das_CPP schrieb:

--> Warum ist das nicht gut?

Weil es sinnfrei ist? Du verwaltest hier keinen typenlosen Speicher, sondern Klasseninstanzen, also hat void* hier nichts verloren.
Wenn es unbedingt Zeiger sein müssen, dann so:

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

Ich hoffe du erwartest dir keinen Kommentar dazu, warum man Memberfunktionen nicht mit Makros, sondern mit... naja, eben mit Memberfunktionen realisiert.
Allerdings kannst du dir die ohnehin sparen, denn wenn du der Außenwelt zügellosen Zugriff auf deine Klasseninterna gibst, brauchst du gleich keine Interna und kannst die auch public machen.

und was ist mit der Funktionsmakros?

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

Soll ich weiterhin die Funktionsmakro verwenden?
warum ist das nicht zu empfehlen Funktionsmakro zu verwenden?

Danke

SeppJ

Ist_Das_CPP schrieb:

warum ist das nicht zu empfehlen Funktionsmakro zu verwenden?

Weil sie alle Nachteile von Makros haben und gegenüber Funktionen überhaupt gar keinen Mehrwert?

Die wahre Lösung deines Problem ist aber, das ganze kaputte Design weg zu werfen, anstatt zu versuchen, die Bruchbude irgendwie notdürftig mit etwas frischer Farbe zu renovieren.

RHBaum

Weil sie alle Nachteile von Makros haben und gegenüber Funktionen überhaupt gar keinen Mehrwert?

Warum macht man dann sowas freiwillig ?

Ich würd mal fragen,
1. wie alt der Code ist ?
2. in welchem Context er verwendet wird ...

Ich mein void * hin und her zu casten um die typsicherheit und nebenbei alle Annehmlichkeiten einer IDE in der Beziehung zu verlieren, da muss die Not schon groß sein ... oder es war wirklich ne sehr verirrte Seele ?

Für mich siehts so aus, als würden hier C++ Schnittstellen nachträglich ohne viel Aufwand versucht auf C Schnittstellen zu portieren. Die defines könnten versuchen, die Im und Export Makros der exportierten funktionen zu "zentralisieren", bzw makros an und abzuschalten. Gibst von den Makros weitere "varianten" ? Ist natürlich aber auch nur ne Vermutung.

Ciao ...

RHBaum schrieb:

Weil sie alle Nachteile von Makros haben und gegenüber Funktionen überhaupt gar keinen Mehrwert?

Warum macht man dann sowas freiwillig ?

Dieses Frage kann ich dir ehrlich gesagt gar nicht beantworten
ich muss damit zu recht kommen mit dem was ich habe

Ich würd mal fragen,

RHBaum schrieb:

1. wie alt der Code ist ?

Ich schätze mal 5 Jahre und bin seit 1 Jahr hier
also es gibt viel aufzuräumen

RHBaum schrieb:

2. in welchem Context er verwendet wird ...

Es ist als Library "DLL" verwendet.

RHBaum schrieb:

Ich mein void * hin und her zu casten um die typsicherheit und nebenbei alle Annehmlichkeiten einer IDE in der Beziehung zu verlieren, da muss die Not schon groß sein ... oder es war wirklich ne sehr verirrte Seele ?

Für mich siehts so aus, als würden hier C++ Schnittstellen nachträglich ohne viel Aufwand versucht auf C Schnittstellen zu portieren. Die defines könnten versuchen, die Im und Export Makros der exportierten funktionen zu "zentralisieren", bzw makros an und abzuschalten. Gibst von den Makros weitere "varianten" ? Ist natürlich aber auch nur ne Vermutung.
Ciao ...

Diese Makros würde ich auch nicht verwenden - möglichst sogar entfernen.
Nimm static_cast<xyz>() das versteht jeder.

An den void* würde ich nix ändern - solange Du nicht weisst, was da die Motivation war.

Sowas kommt in den besten Familien vor

Siehe z.B. hier https://github.com/boostorg/iostreams/blob/develop/src/bzip2.cpp
bz_stream ist ein void* (siehe bzip2_base https://github.com/boostorg/iostreams/blob/develop/include/boost/iostreams/filter/bzip2.hpp)
und muss ständig gecastet werden.

Wenn ich so schreibe, habe ich eingentlich nicht viel davon

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

und

Bclass* pmaGetBclass(){
  return m_pvBclass;
}

DclassPool* pmaGetDclassPool(){
  return m_pvDclassPool;
}

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

Es funktioniert, aber es muss ständig aufgeraumt werden.

Wie sieht es aus wenn ich mit der neuen c++11 Standard zu Hilfe nehme?

Deklationen der Zeiger wird es so aussehen:

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

Hallo,

ich stehe wieder auf dem Schlauch.
Wenn ich meiner Zeiger so deklariere:

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

Wie sollen den die :

pmaGetBclass()

,

pmaGetDclassPool()

und

pmaGetEclassHold( int a )

implementiert werden?

RHBaum

Deshalb frag ich nach dem Context ...

boost::shared_ptr<Bclass> super Idee, wenn:

Die Definition nicht deine Übersetzungseiheit verlässt.
Also sowas über Dll Grenzen zu exportieren wird zur Abhängigkeitshölle und du verlierst jeglichen "Vorteil" einer Dll ....

Super 100% Dll Lösung (Binärkompatiblität auf Lebenszeit) = reine C Interfaces.
Hier werden alle Methoden einer Klasse zu normalen globalen funktionen und der Klassencontext zum Handle, welche Funktion(methode) zu welcher Klasse(handle typ) gehört, ist meist im namen Codiert
Rueckgabecodes meist erforderlich, da exception Handling ueber Dll grenzen auch oft ne miese Idee ist ...

Creator Funktionen sehen dann so aus
int Create_Typ_Irgendwas(void ** handlePtr,/* ... params ... */);
Die Ähnlichkeit zu deiner Vorlage kommt Imho nicht von ungefähr ^^

die KomfortLösung : nur 100% abstrakte Klassen (interface) über dll grenzen definieren. Dann ist man nur von der Definition der vtables abhängig, aka, die meisten Compilerhersteller Halten das compatibel, ist aber kein Muss.

Was gar nicht geht = templates über Dll interfaces. Dann wirst du bei jedem neukompilieren deiner Anwendung die Dll fast immer mit neu bauen müssen.
Eine Dll von mehreren Apps verwenden wird damit ... schwierig.
And der Stelle bieten sich dann statische Libs eher an.

Also wenn deine Vorgabe ne Dll Schnittstelle ist, veruch nicht deine Lösung in super modernen C++ Sprachmitteln zu finden. C++ = hochgradig binär incompatibel. und Kompatiblitaet ist bei ner Dll eigentlich schon prinzipiell ein Thema ...

Ciao ...

Hi danke für die Hinweis.

Ich werde ersmal vorüber gehen dieses vorschalge einfach so lassen und step für step das ganze Projektaufbau neu deignen:

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

und

Bclass* pmaGetBclass(){
  return m_pvBclass;
}

DclassPool* pmaGetDclassPool(){
  return m_pvDclassPool;
}

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

es ist zwar mit sehr viel unschönheit aber immerhin besser als dieses FKT makros

Ja aber nicht vergessen delete am ende