Code Verbesserungen

nemomuk

cooky451 schrieb:

Beim schnellen durchlesen ist mir aufgefallen:
Zeile 13 + 20: Warum steht die Klammer denn nicht ganz links?

hm, der Einheit halber könnte man es nach links schieben ja

cooky451 schrieb:

Zeile 27: Irgendwie unschön.. warum arbeitest du nicht einfach mit der als Parameter übergebenen Node? Bist du dir sicher, dass das überhaupt das macht was du da erreichen willst? (Ok, könnte sogar funktionieren, aber unschön bleibts :D)

Ja stimmt, das Anlegen eines neuen Nodes ist überflüssig.

cooky451 schrieb:

Zeile 130 ff: Das kompiliert so?

Laut Google kompiliert das nur mit GNU C und und C99. Fand es schöner in eine Zeile zu packen.

Danke schonmal für deine Tipps! Freue mich auch auf weitere Kritikpunkte.

nemomuk

ghjghjhgj schrieb:

Dein Programm ist nicht unbedingt gültiges C. Variablendeklarationen müssten nämlich dann immer am Blockanfang stehen, was bei dir nicht der Fall ist.

Heißt: sowas wie int i; etc. muss immer an den Blockanfang?

nemomuk

@cooky451: Ich habe versucht, das hier umzuschreiben und direkt den bereits erstellten Node zu verwenden (nodes[i].dist = DBL_MAX), was aber leider nicht funktioniert:

void init_nodes(Node *nodes) {
    int i;
    for (i=0; i<NODE_NUMBER; i++) {
        // node names beginning with A + i
        Node node = {{65 + i}, DBL_MAX};
        nodes[i] = node;
    }
}

Wie würde man das dann machen?

linux_c89

Node node = {{65 + i}, DBL_MAX};

Erstens: schreibe doch bitte 'A'+i, Das funktioniert auch bei nicht-ASCII-Systemen (weniger wichtig) und ist besser lesbar(sehr wichtig)
Zweitens: Das passt irgendwie nicht zu deiner Strukturendefinition, ist das richtig so? (Ich benutze diese {} schreibweise eig. nur bei konstanten Initialisierungen, deshalb kenne ich mich da nich so aus)

cooky451

ahojnnes schrieb:

Heißt: sowas wie int i; etc. muss immer an den Blockanfang?

Seit C99 eigentlich nicht mehr, aber da der Visual Studio C Compiler das immer noch nicht kann, ist es oft üblich in C alles explizit am Anfang (also vor dem ersten Funktionsaufruf, der keine Initialisierung darstellt) zu deklarieren.

nodes[i].dist = DBL_MAX; hat nicht funktioniert
Was kommt denn für eine Fehlermeldung?

cooky451

linux_c89 schrieb:

(Ich benutze diese {} schreibweise eig. nur bei konstanten Initialisierungen, deshalb kenne ich mich da nich so aus)

Es funktioniert vermutlich da alles andere mit Nullen initialisiert wird, damit ist sogar der "String" Nullterminiert, aber es ist schon sehr dreckig gehackt

nemomuk

sry, habe mich falsch ausgedrückt: nodes[i].dist = XX funktioniert, allerdings weiß ich nicht wie ich das name initialiseren soll mit dem {'A' + i}?

void init_nodes(Node *nodes) {
    int i;
    for (i=0; i<NODE_NUMBER; i++) {
        nodes[i].name[0] = (char) ('A' + i);
        nodes[i].dist = DBL_MAX;
    }
}

Bekomme so einen Segmentation fault und bei der Ausgabe ziemlich kryptische Zeichen.

Danke!

cooky451

Ein String ist in C halt ein char Array mit einer 0 am Ende.

nodes[i].name[0] = 'A' + i;
nodes[i].name[1] = '\0';

Die Nullterminierung ist sehr wichtig und führt gerade bei Anfängern zu Fehlern. (z.B. weil in ein char buf[5]; eben kein "hallo" passt ;))

nemomuk

ah, gut zu wissen. Vielen Dank!

Sonst keine Vorschläge oder Stichworte/Themen, in die ich mich einlesen könnte?

Bin momentan noch etwas erstaunt, wie fragil das ganze ist, wenn ich nur an einem Punkt eine Speicherstelle falsch anspreche. Kann man da in C den Prozess nicht etwas mehr "abkapseln" und mit return values arbeiten?

cooky451

ahojnnes schrieb:

Bin momentan noch etwas erstaunt, wie fragil das ganze ist, wenn ich nur an einem Punkt eine Speicherstelle falsch anspreche.

Das stimmt, bringt C so mit sich. Allerdings fällt so was eigentlich immer schnell auf. Schlimmer wirds dann mit malloc() und free()

ahojnnes schrieb:

Kann man da in C den Prozess nicht etwas mehr "abkapseln" und mit return values arbeiten?

Wie genau meinst du das?

nemomuk

ich dachte daran, dass ich einer Funktion z.B. ein array übergebe und ein neu angelegtes array zurückbekomme.

Für welche Zwecke benutzt man dann malloc und free?

cooky451

ahojnnes schrieb:

ich dachte daran, dass ich einer Funktion z.B. ein array übergebe und ein neu angelegtes array zurückbekomme.

Ich versteh immer noch nicht genau, was das bringen soll.. könntest du vielleicht mal einen Anwendungsfall beschreiben / Peusocode zeigen? Ein Array zurückgeben wird schwer

ahojnnes schrieb:

Für welche Zwecke benutzt man dann malloc und free?

Um sich Speicher vom Heap zu reservieren. Ein z.B. 5 MB großes Bild passt üblicherweise nicht auf den Stack.

nemomuk

In meinem Beispiel fände ich es zB logischer, wenn die Funktion init_nodes nicht ein array modifiziert, sondern ein neu erstelltes array mit variabler Länge zurückliefert. Somit würde ich auch von dem statischen Wert NODE_NUMBER wegkommen. Das Ganze funktioniert im Moment ja nur mit einer statischen Graphen Struktur.

cooky451

ahojnnes schrieb:

In meinem Beispiel fände ich es zB logischer, wenn die Funktion init_nodes nicht ein array modifiziert, sondern ein neu erstelltes array mit variabler Länge zurückliefert.

Würde ich nicht logischer finden. Du könntest zwar in der Funktion dynamisch Speicher anfordern (malloc ;)) und dann einen Zeiger auf den Speicherbereich zurückgeben, aber das ist zu recht eher unüblich. Denn man vergisst dann schnell mal, so etwas wieder frei zu geben. Und der Stack ist hier eh schneller.

ahojnnes schrieb:

Somit würde ich auch von dem statischen Wert NODE_NUMBER wegkommen.

Übergib der Funktion doch einfach die Größe:

ahojnnes schrieb:

void init_nodes(Node *nodes, int size) {
    int i;
    for (i=0; i<size; i++) {
        nodes[i].name[0] = 'A' + i;
        nodes[i].name[1] = '\0';
        nodes[i].dist = DBL_MAX;
        nodes[i].visited = 0;
    }
}

nemomuk

Ok, auch eine Möglichkeit. Gibt es eine Möglichkeit die Länge eines arrays zu bestimmen (wenn es sich nicht um einfache Datentypen wie int etc. handelt), damit man sich das size sparen kann (würde man hier int oder size_t verwenden - im Endeffekt ja das gleiche)?

cooky451

ahojnnes schrieb:

Gibt es eine Möglichkeit die Länge eines arrays zu bestimmen

Ja:
double array[50];
foo(array, sizeof(array) / sizeof(array[0]));
sizeof() liefer die Größe des übergebenen Typs in Byte zurück.

ahojnnes schrieb:

damit man sich das size sparen kann

Das kannst du dir nicht sparen, da Node *nodes ein Pointer und kein Array ist. Node nodes[] klappt im übrigen auch nicht. Das size wirst du wohl mit übergeben müssen.

ahojnnes schrieb:

(würde man hier int oder size_t verwenden - im Endeffekt ja das gleiche)?

size_t ist eher ein "unsigned int". ints sind standardmäßig signed. size_t/unsigned int wäre hier sogar schöner, aber ich bin so schrecklich schreibfaul

nemomuk

alles klar, dann bedanke ich mich für dein Feedback und werde bei Fragen evtl. nochmal auf das Forum hier zurückkommen!

Beste Grüße.