Abreitsschritte zusammenfassen möglich?

Hallo!

ich habe einen kleinen XML Parser geschrieben, der bestimmte Attribute herauslesen soll. Leider wiederholt sich hier einiges, so dass es insgesamt ein wenig unschön aussieht. Auch wird die Pflege langsam nicht so einfach.

Vielleicht hat jemand einen Ansatz, wie man es zu uusammenfassen könnte.

Ab Zeile 13-44 ist fast das selbe wie 45-76 und 77-104

bool Xmlparse::set_obj_prop(Xmlobject &obj,wxXmlNode *node){

	long tmplong;
	bool flag[15];
	for(int i=0;i<sizeof(flag)/sizeof(bool);i++) flag[i] = true;

	wxXmlProperty *prop = node->GetProperties();

	assert(flag[0]);
	while(prop&&flag[0]){

		tmplong = -1;

		if(prop->GetName()==_("size")){

			assert(prop->GetValue().ToLong(&tmplong));
			if(!prop->GetValue().ToLong(&tmplong)){
				flag[0] = false;
				flag[1] = false;
				break;
			}

			assert(tmplong>0);
			if(tmplong<1){
				flag[0] = false;
				flag[2] = false;
				break;
			}

			prop = prop->GetNext();

			assert(node->DeleteProperty(_("size")));
			if(!node->DeleteProperty(_("size"))){
				flag[0] = false;
				flag[3] = false;
				break;
			}

			assert(obj.set_size(tmplong));
			if(!obj.set_size(tmplong)){
				flag[0] = false;
				flag[4] = false;
				break;
			}
		}
		else if(prop->GetName()==_("code")){

			assert(prop->GetValue().ToLong(&tmplong));
			if(!prop->GetValue().ToLong(&tmplong)){
				flag[0] = false;
				flag[5] = false;
				break;
			}

			assert(tmplong>0);
			if(tmplong<1){
				flag[0] = false;
				flag[6] = false;
				break;
			}

			prop = prop->GetNext();

			assert(node->DeleteProperty(_("code")));
			if(!node->DeleteProperty(_("code"))){
				flag[0] = false;
				flag[7] = false;
				break;
			}

			assert(obj.set_code(tmplong));
			if(!obj.set_code(tmplong)){
				flag[0] = false;
				flag[8] = false;
				break;
			}
		}
		else if(prop->GetName()==_("color")){

			if(!prop->GetValue().ToLong(&tmplong)){
				flag[0] = false;
				flag[9] = false;
				break;
			}

			if(tmplong<1){
				flag[0] = false;
				flag[10] = false;
				break;
			}

			prop = prop->GetNext();

			if(!node->DeleteProperty(_("color"))){
				flag[0] = false;
				flag[11] = false;
				break;
			}

			if(!obj.set_color(tmplong)){
				flag[0] = false;
				flag[12] = false;
				break;
			}
		}

		else if(prop->GetName()==_("comment")){

			if(!set_comment(prop->GetValue().mb_str())){
				flag[0] = false;
				flag[13] = false;
				break;
			}

			prop = prop->GetNext();

			if(!node->DeleteProperty(_("comment"))){
				flag[0] = false;
				flag[14] = false;
				break;
			}
		}
		else prop = prop->GetNext();
	}
// Flags auswerten
	return flag[0];
}

Mir ist nun doch etwas eingefallen.
Ich bilde ein Array in dennen ich die Attribute speichere und iteriere mich durch diese Arrays.

Leider klappt es mit den Funktionszeigern in Zeile 9 nicht. Kann mir jemand ein Tipp geben?

bool Xmlparse::set_obj_prop(Xmlobject &obj,wxXmlNode *node){

	long tmplong;
	bool flag[7];

	wxString attribute[] = {_("size"),_("code"),("color")};

	typedef bool(*pfunctions)(puint);
	pfunctions set_function[] = {&set_size(), &set_code(),&set_color()};

	for(int i=0;i<sizeof(flag)/sizeof(bool);i++) flag[i] = true;

	wxXmlProperty *prop = node->GetProperties();

	assert(flag[0]);
	while(prop&&flag[0]){

		tmplong = -1;

		for(int i=0;i<sizeof(attribute)/sizeof(wxString);i++){

			if(prop->GetName()==attribute[i]){

				assert(prop->GetValue().ToLong(&tmplong));
				if(!prop->GetValue().ToLong(&tmplong)){
					flag[0] = false;
					flag[1] = false;
					break;
				}

				assert(tmplong>0);
				if(tmplong<1){
					flag[0] = false;
					flag[2] = false;
					break;
				}

				prop = prop->GetNext();

				assert(node->DeleteProperty(attribute[i]));
				if(!node->DeleteProperty(attribute[i])){
					flag[0] = false;
					flag[3] = false;
					break;
				}

				assert(set_function[i](tmplong));
				if(!set_function[i](tmplong)){
					flag[0] = false;
					flag[4] = false;
					break;
				}
			}
		}

		if(prop->GetName()==_("comment")){

			if(!obj.set_comment(prop->GetValue().mb_str())){
				flag[0] = false;
				flag[5] = false;
				break;
			}

			prop = prop->GetNext();

			if(!node->DeleteProperty(_("comment"))){
				flag[0] = false;
				flag[6] = false;
				break;
			}
		}
		else prop = prop->GetNext();
	}
	return flag[0];
}

könnte mir jemand einen Tipp geben wie ich ein Array aus Funktionszeigern anlegen kann.

// die funktionen
//Aus der Klasse Xmlobject
bool set_size(long);
bool set_code(long);
bool set_color(long);

Normalerweise müsste doch so etwas funktionieren?

typedef bool(Xmlobject::*xmlfunc)(long);
xmlfunc set_functions[3] = {obj.set_size(), obj.set_code(),obj.set_color()};

Und der Aufruf

if(!set_functions[i](tmplong)){
	flag[0] = false;
	flag[i] = false;
	break;
}

anfänger_ schrieb:

könnte mir jemand einen Tipp geben wie ich ein Array aus Funktionszeigern anlegen kann...

Habs herausgefunden. Funktioniert einwandfrei.

Warum bekam ich keine einzige Antwort? Ist der Code so falsch?

anfänger_ schrieb:

Warum bekam ich keine einzige Antwort? Ist der Code so falsch?

Ich vermute mal, weil den meisten hier nicht klar ist, warum du die ganzen Flags
eigentlich setzt...

Ich versteh ehrlich gesagt auch nicht so ganz, was du vor hast...

Ab Zeile 13-44 ist fast das selbe wie 45-76 und 77-104

Dann kuck doch mal, ob du es in eine Funktion auslagern kannst...
Ich denke schon.
Alles was unterschiedlich ist, übergibst du als Parameter, wobei du
beim array sowas wie flag[5] übergeben kannst, dann kannst du in der
Funktion immer die gleichen Indizes verwenden.
flag[0] mußt du dann natürlich anders behandeln

Gruß,
CSpille

SeppJ

anfänger_ schrieb:

Warum bekam ich keine einzige Antwort?

Langer, (extrem!) kryptischer Code 70 % der Leser lesen gar nicht erst weiter
Nur vage Beschreibung was das Codestück tun soll Nochmals ein großer Teil der Leserschaft ist abgeschreckt
Wer also nicht durch den Code abgeschreckt ist, findet:
Sehr vage Fragestellungen Wer jetzt noch mit liest, müsste erstmal deinen ganzen Code entschlüsseln (so muss man das nennen), dein konkretes Problem erraten und noch eine passende Antwort suchen.

All dies riecht nach 30-60 Minuten Arbeit für eine eher uninteressante Fragestellung. So gutmütig ist niemand.

Wie man es besser machen könnte:
Dem Leser unnötige Denkarbeit abnehmen. Code auf etwas äquivalentes, verständliches umformulieren. Dazu eine konkrete Problembeschreibung und vielleicht schon einen Ansatz zur Problemlösung und eine konkrete Frage warum das nicht klappt.

Zu deiner Problemstellung: Wenn ich deinen Code richtig verstanden habe, dann kann man zum Beispiel Zeilen 24 bis 52 abkürzen zu:

assert(prop->GetValue().ToLong(&tmplong));
assert(tmplong>0);
prop = prop->GetNext();
assert(node->DeleteProperty(attribute[i]));
assert(set_function[i](tmplong));

edit: Ich habe tatsächlich 26 Minuten für die Antwort gebraucht. Verdammt!