Wie kann ich meinen Code verbessern?

[DirkB]

Pack doch die globalen Variablen in eine struct . Dann hast du nur einen extra Parameter.

Namen mit beginnenden Unterstrich sind für den Compiler/System reserviert.

[querasd]

theSplit schrieb:

- Namens niemals mit Unterstrich beginnen lassen

Was wäre der Grund hierfür?

Namen die mit einem Underscore und einem Uppercase Buchstaben folgen sind für den Compiler reserviert. Das gleiche gilt für Namen, die mit mehr als einem Underscore beginnen. In deinem Fall ist es also noch ok.

[Wutz]

- viele FILE-orientierte Funktionen liefern die EOF Informationen im return-Wert mind. implizit mit
- der rohe realloc-Gebrauch ist bei häufigem Aufruf suboptimal
- insgesamt zuviel Code, wird unübersichtlich und damit fehleranfällig
- wiederholte Elemente in mehreren struct
- üblicherweise vermeidet man lange Parameterlisten durch ein struct

typedef struct {
char **argv;
int bla, return;
char filename[FILENAME_MAX];
...
} Konfiguration;

void init(Konfiguration *k){...}
void run(const Konfiguration *k){...}
void done(const Konfiguration *k){...}

int main(int argc,char**argv)
{
  Konfiguration k={argv};
  init(&k);
  run(&k);
  done(&k);
  return k.return;
}

Du kannst so schon durch das Design entscheiden, dass der Compiler dich auf Fehler hinweist, hier also z.B. wenn du nach der Initialisierungsphase noch die Parameter veränderst, die aber in der Phase tabu sein sollen.
Außerdem lassen sich durch solche flexiblen Konfigurationsobjekte sehr viel besser Testfälle schreiben.

[theSplit]

Hallo.

Ich bin jetzt noch ein paar eurer Vorschläge nachgegangen:
Die globalen Variablen für die Dateistatistik sind jetzt in einem Struct zu finden das an die Funktionen per Referenz übergeben wird.

Die Entities sind jetzt in Escape Sequenzen und nicht mehr direkt als Unicode-Zeichen eingebunden.

Alle mit #define deklarierten Zähler für die konstanten Datentypen und Switches/Parameter werden in Uppercase geschrieben.
Die Switches sollen ja noch komplett verschwinden, aber zum aktuellen arbeiten mit dem Code ist das setzen mit #define erstmal sehr viel leichter, wenn auch nicht zwingen schön. Aber das würde ich aktuell vernachlässigen wollen.

--

Ansonsten überlege ich gerade über die Wutz Vorschläge nach die Variablen besser zu benennen. Damit tue ich mir allerdings schwer und würde mal gerne in die Runde fragen:
Wie handhabt ihr in der Regel die Namens bzw. die Schreibweise (Uppercase, CamelCase, Lowercase) von Variablen, Funktionen und Konstanten? Gibt es dafür ein paar definierte Vorgaben? Gibt es da Coding Conventions in C denen man folgen kann bzw. die man sich mal anschauen kann?

Zweiter Punkt an dem ich noch hadere, die Konfiguration des Programms als Struct zu übergeben.
Ich hab mal gelesen das man nicht mehr als 3 Parameter an eine Funktion übergeben lassen sollte, weil sonst so viel auf den Stack kopiert werden muß wenn die Funktion aufgerufen wird und das für die Performance nicht förderlich wäre.
Frage wäre an auch jetzt, ist das (immernoch) so? Habe ich etwas falsches gelesen? Ist das zu vernachlässigen oder dient es eigentlich mehr der Übersicht damit die Funktionsparameter innerhalb der "80 Zeichen" Platz finden und nicht die Ansicht sprengen (wie es momentan bei einigen Funktionen der Fall ist) ?

[dachschaden]

theSplit schrieb:

Wie handhabt ihr in der Regel die Namens bzw. die Schreibweise (Uppercase, CamelCase, Lowercase) von Variablen, Funktionen und Konstanten? Gibt es dafür ein paar definierte Vorgaben? Gibt es da Coding Conventions in C denen man folgen kann bzw. die man sich mal anschauen kann?

Solange es nur lesbar ist, ist es eigentlich ziemlich egal, glaube ich.
Persönlich habe ich das Pascal-Case der WinAPI (z.B. RegEnumValue ) immer als "ordentlich" empfunden, aber programmieren tue ich trotzdem in ... naja, ich weiß nicht, wie man das nennt. Ich programmiere einfach meiner Erfahrung nach. winapi_registry_process_stub , sowas halt. Aber da denke ich nicht bewusst drüber nach, und das solltest du auch nicht, solange es keine Vorgabe ist.

theSplit schrieb:

Ich hab mal gelesen das man nicht mehr als 3 Parameter an eine Funktion übergeben lassen sollte, weil sonst so viel auf den Stack kopiert werden muß wenn die Funktion aufgerufen wird und das für die Performance nicht förderlich wäre.

Halte ich für Unsinn. Funktionen mit vielen Parametern können dir helfen, deinen Code unglaublich allgemein und klein zu halten - als Beispiel eine Funktion, die in der Lage sein soll, auf Windows und Linux Verzeichnisse durchzugehen und Eintragstrings zwischenzuspeichern. Die hat elf Parameter und spart mir woanders einen Haufen explizit auf eine bestimmte Funktionalität ausgerichteten Code.

Problematisch wird es, wenn du spezifiziert, was für Parameter übergeben werden. Nur Zeiger auf Objekte? Am Besten noch als const deklariert, weil nur von diesen gelesen werden soll? Kein Problem, ein Zeiger ist verhältnismäßig klein (4 Bytes auf 32-Bit/8 Bytes auf 64-Bit-System). Anders schaut's aus, wenn du ganze Objekte auf den Stack legen willst - ein Objekt, welches in der Lage sein soll, alle (!) möglichen Socket-Adresstypen zu speichern? Da sind wir bei mindestens 128 Bytes pro Objekt. Da komm ich mit meinen elf Zeigern (88 Bytes) gar nicht an.

Bei rekursiven Funktionen möchtest du den Footprint auf den Stack auch eher klein halten.

Wenn du's allgemeiner halten willst, kannst du immer noch Macros definieren, die im Hintergrund deine Funktion aufrufen, aber einen Teil der Parameter bereits so übergeben. Wie die Standardparameter in C++. Mach ich auch ständig. So häufig sogar, dass ich Macros habe, die mir Werte generieren, die ich dann in Macros verwende, damit diese dann Funktionen aufrufen. Am Ende hast du dann nur ein http_stack_init_simple(hst) und siehst den Aufruf von http_stack_init gar nicht:

#define http_stack_init_simple(hst) \
    http_stack_init((hst),0,NULL,NULL,FALSE,0,0,0)

Ein paar Programmierer sagen, dass man dann lieber eine Parameterstruktur bauen und davon ein Objekt anlegen soll, dem man dann die Parameter übergibt. Kann man machen. Aber in der Realität hatte ich noch keine Funktionen, wo dies den Code lesbarer gemacht hätte. Und es macht auf wunderbare Weise meine Standardparametermacros unnötig kompliziert, manchmal sogar kaputt [wie bekommt man den Rückgabewert eines compound statements in einem Macro zurück? Antwort: offiziell gar nicht. Inoffiziell können einige Compiler (GCC) das, aber es bricht dann mit anderen Compilern, und das ist blöd].

Und soweit geht selbst Microsoft nicht so oft.

[theSplit]

dachschaden schrieb:

theSplit schrieb:

Wie handhabt ihr in der Regel die Namens bzw. die Schreibweise (Uppercase, CamelCase, Lowercase) von Variablen, Funktionen und Konstanten? Gibt es dafür ein paar definierte Vorgaben? Gibt es da Coding Conventions in C denen man folgen kann bzw. die man sich mal anschauen kann?

Solange es nur lesbar ist, ist es eigentlich ziemlich egal, glaube ich.
Persönlich habe ich das Pascal-Case der WinAPI (z.B. RegEnumValue ) immer als "ordentlich" empfunden, aber programmieren tue ich trotzdem in ... naja, ich weiß nicht, wie man das nennt. Ich programmiere einfach meiner Erfahrung nach. winapi_registry_process_stub , sowas halt. Aber da denke ich nicht bewusst drüber nach, und das solltest du auch nicht, solange es keine Vorgabe ist.

Naja, die Idee feste Konventionen zu haben macht ja schon Sinn.
Wenn ich deine Schreibweise winapi_registry_process_stub für Funktionen verwenden würde, wäre sofort ersichtlich das jetzt eine Funktion kommt.

Haben wir eine vordefinierte Konstate und diese wird in UPPERCASE geschrieben, wird dies sofort ersichtlich und man rührt diese erst gar nicht bzw. sieht dass man diese weiterverwenden kann.

Ansonsten durchzieht meinen Code bisher nur camelCase - das habe ich mir so angewöhnt.

Aber eine Trennung macht, schon irgendwo Sinn. Ist natürlich etwas doof wenn es dafür keine guten Coding-Standards gibt an denen man sich orientieren kann.
Ich glaube es gab da mal eine Seite Ninja-Coding oder Karate-Coding, irgend so etwas, auf denen wurde auch auf Conventions eingangen...

Wenn man sowas durchgängig hält, kann ich mir schon vorstellen dass dies vieles vereinfacht als wenn man das dem Zufall überlässt.

Naming Conventions machen ja auch Sinn, so weiß ich gleich durch "isValid" oder "hasToken" das es sich zu 99,9 % um boolsche Werte handelt - ohne überhaupt nachvollziehen zu müssen wie und wo die Variable ihre Werte her bekommt, es wird aus dem Kontext eigentlich relativ klar.
Sollte man zwar auch prüfen, aber ich meine sowas ist gängige Praxis.

[Wutz]

theSplit schrieb:

Wutz schrieb:

- zum XML-Parsen nimmt man fertige Bibliotheken

Das XML Parsen ist eigentlich keine Schwierigkeit, das was so umfangreich ist sind die Daten einzulesen.

Der eigentliche XML Parser ist ~60 bis 90 Zeilen lang, warum sollte ich dafür eine Bibliothek verwenden wollen? Jedenfalls für diesen Einsatzzweck? Macht für mich relativ wenig Sinn, ich sehe nicht zwingend die Vorteile, außer meine Implementierung würde nicht funktionierten - aber sie tut scheinbar das was sie soll, einlesen...

Eine Art SAX Parser wäre auch ideal, ich habe aber im Kopf so eine Blockade das ich unbedingt über den gesamten Dateiaufbau bescheid wissen muß, um damit dann später etwas optimiertes ausgeben zu können was die gleiche Struktur besitzt.

Auch für SAX-Parser gibt es fertige Bibliotheken, sogar ziemlich viele.
Ich bleibe dabei, man sollte fertige und gut abgehangene Bibliotheken nehmen.
Und die sollten auch nicht gleich abstürzen, wenn man ihnen - wie du vorhast - XML-Dokumente mit fehlerhafter Baumstruktur vorsetzt, und daran wärst auch du gescheitert.

Man sollte solchen Bibliotheken aber auch nicht kritiklos vertrauen, und mit einiger Erfahrung darf man dann die Quellcodes auch mal durchblättern.
Vor vielen Jahren habe ich mal xercesc verwendet, damals war es noch eine reine C-Bibliothek, heute mit einem häßlichen C++ Überbau. Keine Ahnung, warum das sein musste, schließlich werkeln in der Basis nach wie vor reine C-Codes.

Nachdem ich dann zufällig auch noch mal in den gehypten libxml2 Code geschaut habe, und mir dabei schlecht wurde, hab ich mir gesagt: das kann ich besser.

Herausgekommen ist dann:
https://github.com/cdoyen/xmlclean

Dabei sieht man dann: Portabilität und Performanz müssen sich nicht ausschließen.
Es müssen auch keine globalen Variablen, Makros und #ifdefs sein, und trotzdem läuft es überall und performant.
Und mit Callbacks kann man schon eine Menge machen, insbesondere die API einer Bibliothek übersichtlich und flexibel gestalten.

[theSplit]

Mh, interessant.

Ich habe allerdings ziemliche Probleme deinen Quellcode nachzuvollziehen, weil mir die Variablen, alles so verdammt gekürzt, einfach nicht plausibel erscheinen und ich nicht verstehe was hier und dort angesprochen wird.
Ich finde da könntest du vielleicht nochmal etwas tun was die Benennung anbelangt, auch wenn der Code dann nicht so "super kompakt" ist...

Was mir auch gerade fehlt, wie sehen die Daten aus, die deine Bibliothek zurück gibt an den Aufrufer?

Aus einer Readme deines Projekts:

Ausgehend vom Beispiel

<?xml version="1.0"?>
<!--Introduction-->
<foo>
<bar>baz</bar>
<baz foo/>
</foo>Müll<Schrott

ergeben sich dann die Typen und Wertepaare wie folgt:

NORMALCLOSE_ : "baz" + "/bar"
OPENTAG_     : "\n" + "foo" , "\n" + "bar"
SELFCLOSE_   : "\n"  +"baz foo/"
FRAMECLOSE_  : "\n" + "/foo"
COMMENT_     : "\n"  +"!--Introduction--"
PROLOG_      : "" + "?xml version="1.0"?"
UNKNOWN_     : "Müll " + "Schrott"

Aber wie wäre jetzt der Anwendungsfall? - Bezieht sich das ausschließlich auf das "Bereinigen" von XML über die Callbacks? Oder erhalte ich Informationen über den Aufbau zurück?

Wäre nett wenn du mir meine (etwas naiven) Fragen beantworten könntest.

[Wutz]

Der Anwendungsfall ist ganz einfach und z.B. im Code bei den vorgegebenen Default-Callbacks oder im Beispiel xmlclean:worker_own ablesbar:

#include "xmlclean.h"
#include <stdio.h> /* fopen,fclose,fprintf,stderr */

int worker_own(int typ, const unsigned char *tag, size_t taglen, int out(), void* f, Parser *p)
{
	switch (typ)
	{
	case NORMALCLOSE_:
		break;
	case OPENTAG_:
		break;
	case SELFCLOSE_:
		break;
	case FRAMECLOSE_:
		break;
	case COMMENT_:
		break;
	case PROLOG_:
		break;
	case UNKNOWN_:
		break;
	}
	printf("%d: \"%.*s\" + \"%.*s\"\n", typ, p->contentlen, p->content, taglen, tag);
	return XML_OK;
}

int main(int argc, char**argv)
{
	FILE *f = fopen(argv[1], "rb");
	if (!f) return perror(argv[1]), 1;
	{
		Parser p = { f, 0, worker_own };
		{
			int r = parse(&p);
			if (r)
			{
				if (r == ERRMEM)
					fprintf(stderr, "Fehler Speichermangel");
				else
					if (r == ERRHIERAR)
						fprintf(stderr, "Fehler Hierarchie");
					else
						fprintf(stderr, "Fehler Output");
			}

			fclose(f);

			done(&p);
			return r;
		}
	}
}

Hier siehst du beispielhaft (zugegebenermaßen mit printf und somit an der Callback-API vorbei), wie o.g. Datenpaare zu verstehen und zu benutzen sind.
Im Gegensatz zum SAX-"Standard" gibt es hier bei xmlclean detailliertere Infos für jedes content+tag Paar, eben die Typen und auch die Möglichkeit, viá XPATH die Tags zu separieren.
Die Variablennamen werde ich sicher nicht umbenennen, da du als Anwender ja nicht im Bibliothekscode rumfrickeln sollst, sondern alles über die Initialisierung des Parserobjektes und in den eigenen Callbacks machen sollst, und dort ist alles extensiv benamst und auch dokumentiert.

[swapper]

Wutz schrieb:

- alle globalen Variablen müssen weg

ist nicht einsehbar, wenn du keine plausible begründung lieferst.

[swapper]

dachschaden schrieb:

Halte ich für Unsinn. Funktionen mit vielen Parametern können dir helfen, deinen Code unglaublich allgemein und klein zu halten ...

naja, ab 5 params würde ich ne struct dafür nehmen.

[Wutz]

Depp. Klappe halten, weil du keine Ahnung hast.

[swapper]

Wutz schrieb:

Depp. Klappe halten, weil du keine Ahnung hast.

hört, hört. unser c-profi meldet sich zu wort.

[Wutz]

Ja genau.
Ich hab Ahnung und du keine. Und deshalb trolle dich dahin, wo du herkamst.

[swapper]

Wutz schrieb:

Ja genau.
Ich hab Ahnung und du keine.

unsinn. du bist nur ein blender, mehr nicht.

[Wutz]

Ein ahnungsloser Depp wie du kann, im Gegensatz zu mir, gar keine qualifizierten Antworten zu einem Thema "Wie kann ich meinen Code verbessern" geben.
Halte die Klappe und trolle dich dahin woher du kamst.

[swapper]

Wutz schrieb:

Ein ahnungsloser Depp wie du kann, im Gegensatz zu mir, gar keine qualifizierten Antworten zu einem Thema "Wie kann ich meinen Code verbessern" geben.

aber ein halbwissender vollpfosten wie du gibt zum teil gute und zum teil unsinnige antworten. beides allerdings mit unglaublicher arroganz gewürzt. das ist viel schlimmer, denn so hält der noob auch deinen bullshit für der weisheit letzten schuss. typen wie du sind eine echte plage.

[Wutz]

Dorftrottel wie du die ihr aufgeschnapptes Halbwissen präsentieren, ohne in der Lage zu sein, auch nur annähernd fehlerfreien Code dazu anzubieten sind eine Plage, denn dieses Dorftrottel-Halbwissen zu korrigieren, ist wie man bei deinem Laiengefasel schön sieht, aufwändig.

[swapper]

Wutz schrieb:

Dorftrottel wie du die ihr aufgeschnapptes Halbwissen präsentieren, ohne in der Lage zu sein, auch nur annähernd fehlerfreien Code dazu anzubieten sind eine Plage, denn dieses Dorftrottel-Halbwissen zu korrigieren, ist wie man bei deinem Laiengefasel schön sieht, aufwändig.

das typische rumgezicke eines möchtegern-pros, der wie ein aufgescheuchtes huhn reagiert, wenn mal einer seine zweifelhafte fachkompetenz anzweifelt. was bist du doch für ein armseliger wixer.

[hustbaer]

@Wutz
Du korrigierst doch gar nichts, du beschimpfst bloss.
Was sind die Argumente gegen den Vorschlag von swapper?