Wie kann ich meinen Code verbessern?

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?

Die beiden müssen mal ficken.

theSplit

Bevor der Thread jetzt komplett geschreddert wird, würde ich auch noch meinen Senf dazugeben:

Wutz schrieb:

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.

Also, das "printf" wird so nicht geschluckt, mag der GCC überhaupt nicht.

"printf("%d: \"%ld\" \"%s\"\n", typ, taglen, tag);"
funktioniert.

Allerdings bekomme ich bei meinem Testfile enwik8_small (hier zu finden) einmal einen Format Hierachie Fehler, und "Tag" liefert mir statt eines Tags die Inhalte.

Kannst du das mal bei dir testen?

Außerdem scheint es so, das der aktuelle "gcc" ganz schön viel zu meckern hat an deinem Code.

Zum Beispiel:

xmlclean.c: In function ‘parse’:
xmlclean.c:341:21: warning: ‘taglen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  size_t contentlen, taglen;
                     ^~~~~~
xmlclean.c:341:9: warning: ‘contentlen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  size_t contentlen, taglen;
         ^~~~~~~~~~
xmlclean.c:418:6: warning: ‘tag’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if ((r = worker(UNKNOWN_, tag, taglen, outputcb, outputcbdata, p)) != XML_OK) return r;
      ^
xmlclean.c: In function ‘parse_light’:
xmlclean.c:588:21: warning: ‘taglen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  size_t contentlen, taglen;
                     ^~~~~~
xmlclean.c:588:9: warning: ‘contentlen’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  size_t contentlen, taglen;
         ^~~~~~~~~~
xmlclean.c:638:6: warning: ‘tag’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if ((r = p->worker(UNKNOWN_, tag, taglen, p->outputcb, p->outputcbdata, p)) != XML_OK) return r;

xmltest.c: In function ‘worker_own’:
xmltest.c:23:21: warning: field precision specifier ‘.*’ expects argument of type ‘int’, but argument 3 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
     printf("%d: \"%.*s\" + \"%.*s\"\n", typ, p->contentlen, p->content, taglen, tag);
                     ^
xmltest.c:23:32: warning: field precision specifier ‘.*’ expects argument of type ‘int’, but argument 5 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
     printf("%d: \"%.*s\" + \"%.*s\"\n", typ, p->contentlen, p->content, taglen, tag);

Der Code läuft trotzdem, aber vielleicht kann man das noch ausbügeln.
Bis auf die Formatangaben im "printf".

hustbaer schrieb:

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

Versteh mich nicht falsch, aber wo hat denn swapper einen Vorschlag gemacht? Für mich hat das den Geschmack von subjektiver Code-Gestaltung, mehr nicht. dachschaden hat wenigstens einen Anwendungsfall genannt an dem festzustellen ist, warum er Parameter einzeln übergibt anstatt als struct oder weiss der Geier was.

Dahingegen hat swapper nur gesagt, er würde ab Parameterzahl X ein struct nehmen. Für mich eine Antwort im persönlichen, nicht professionellen Stil.

Wenn swapper einen Vorschlag macht, kann er dies ja auch mit Argumenten belegen, oder etwa nicht? Klar, die Antwort von Wutz hätte auch höflicher gestaltet werden können.

Bierkorken schrieb:

[Dahingegen hat swapper nur gesagt, er würde ab Parameterzahl X ein struct nehmen. Für mich eine Antwort im persönlichen, nicht professionellen Stil.

Also ich hab den Thread jetzt nicht ganz durchgelesen, aber ich persönlich würde immer ein struct nehmen, auch wenn es nur zwei Parameter wären und auch nur wenn die Parameter was miteinander zu tun haben.

theSplit

Pils schrieb:

Bierkorken schrieb:

[Dahingegen hat swapper nur gesagt, er würde ab Parameterzahl X ein struct nehmen. Für mich eine Antwort im persönlichen, nicht professionellen Stil.

Also ich hab den Thread jetzt nicht ganz durchgelesen, aber ich persönlich würde immer ein struct nehmen, auch wenn es nur zwei Parameter wären und auch nur wenn die Parameter was miteinander zu tun haben.

Hi,

ja im Grunde wäre es bei mir im Code auch genau der Fall, ohne diese "Grunddaten" kann keine der Funktionen irgendetwas machen. Hier wäre eine Gruppierung in einem Parameter-Struct vermutlich wirklich sinnvoll.

Auch sollte der Overhead dabei etwas geringer sein einen Parameter statt 5-6-7... zu übergeben? Wie gesagt, ich kann das selbst nicht so einschätzen, ich hatte nur mal gelesen das man Parameterübergaben "kurz" halten/gestalten soll.

Die Idee ein Struct zu verwenden find ich per se auch sehr gut, da man den ganzen "Kontroll-Fluß" bündelt und alle wichtigen Parameter an einem Punkt "abgreifbar" hat, was sich natürlich auch erweitern lässt, ohne an fünf Stellen die Parameter umzuschreiben.

Was mir dabei nur einfällt, wie sieht es aus mit Rekursion? - Wenn ich ein Struct habe das eine Funktion benötigt, aber eine andere Variante wieder an sich selbst übergeben werden muß. Mh - dann wäre das wohl nicht so praktisch?

swapper

Bierkorken schrieb:

Wenn swapper einen Vorschlag macht, kann er dies ja auch mit Argumenten belegen, oder etwa nicht?

ein pointer auf eine struct als parameter hat mehrere vorteile. der funktionsprototyp wird übersichtlicher, die parameter bilden eine logische einheit und die funktion kann z.b. ihr ergebnis in die struct zurückschreiben. das kann man soweit treiben, bis man oop in c hat.

theSplit

swapper schrieb:

Bierkorken schrieb:

Wenn swapper einen Vorschlag macht, kann er dies ja auch mit Argumenten belegen, oder etwa nicht?

ein pointer auf eine struct als parameter hat mehrere vorteile. der funktionsprototyp wird übersichtlicher, die parameter bilden eine logische einheit und die funktion kann z.b. ihr ergebnis in die struct zurückschreiben. das kann man soweit treiben, bis man oop in c hat.

Klingt logisch.