Načtení 2D pole v C

[D.A. Tiger]

Ahoj, pár postřehů z rychlíku

1) Když deklaruješ proměnnou, tak ji co nejdříve také inicializuj a nečekej na chvíli, kdy ji budeš chtít použít. Velmi lehce se na to zapomíná a následky mohou být neblahé. Obzvláště to platí pro pointery!
2) Pokud nejsi schopný zaručit, že ti funkce neselže, pak vždy kontroluj alespoň její návratovou hodnotu, pokud něco vrací. V tomto případě se to týká funkce fopen( ), protože v případě, že se něco provede se souborem, který má otevřít (bude smazán, přejmenován, atd...) koleduješ si - v lepším případě - o segfault o pár řádku níže.

já chápu, že toto je studijní kód začátečníka. Přesto z vlastní zkušenosti považuji za důležité na to upozornit. Důsledky podobných opomenutí se v rozsáhlejším a komplexnějším kódu velmi špatně hledají. A nestačí o tom jen vědět, je potřeba si to vštípit do krve hned od začátku. Věř mi, že ti to hodně časem ulehčí život.

3) Proměnná i je zbytečná. Jenom ji inkrementuješ a a pak přiřadíš do max_i. Popřemýšlej, zda není lepší pracovat rovnou s max_i a i zrušit.
4) Navrhuji ti, abys přesunul podmínku max_j < j až za vnitřní cyklus. To proto, že V současné chvíli testuješ max_j proti všem pozicím elementů v řádku a to v každém cyklu opakovaně. Ale já bych řekl, tebe určitě zajímá, zda max_j neni náhodou menší než celkový počet elementů v řádku. A to budeš vědět až cyklus skončí.   

Tyhle nedostatky přímo funkčnost neovlivní, ale jednak znamenají plýtváním zdrojů mašiny (časem procesoru a pamětí) a jednak hrubě znesnadní čtení kódu v případě rozsáhlejšího souboru. No jen si představ jak by se to četlo, když by jsi měl těch vnořených cyklů několik a byly o něco delší.

5) Pokud vyloženě není tvým záměrem zjišťovat, kolik řádků a kolik elementů v nich je ve vstupu navíc (alespoň v původním zadání o tom nic nebylo), tak já osobně bych z cyklu vyskočil, jakmile bych dosáhl MAX_COLS  nebo MAX_ROWS. Tím si kód zpřehledníš a zjednodušíš, a odpadnou ti ty korekce před výpisem pole.

[Reklama]

[mikrom]

Ahoj, pár postřehů z rychlíku

1) Když deklaruješ proměnnou, tak ji co nejdříve také inicializuj a nečekej na chvíli, kdy ji budeš chtít použít. Velmi lehce se na to zapomíná a následky mohou být neblahé. Obzvláště to platí pro pointery!
2) Pokud nejsi schopný zaručit, že ti funkce neselže, pak vždy kontroluj alespoň její návratovou hodnotu, pokud něco vrací. V tomto případě se to týká funkce fopen( ), protože v případě, že se něco provede se souborem, který má otevřít (bude smazán, přejmenován, atd...) koleduješ si - v lepším případě - o segfault o pár řádku níže.

já chápu, že toto je studijní kód začátečníka. Přesto z vlastní zkušenosti považuji za důležité na to upozornit. Důsledky podobných opomenutí se v rozsáhlejším a komplexnějším kódu velmi špatně hledají. A nestačí o tom jen vědět, je potřeba si to vštípit do krve hned od začátku. Věř mi, že ti to hodně časem ulehčí život.

3) Proměnná i je zbytečná. Jenom ji inkrementuješ a a pak přiřadíš do max_i. Popřemýšlej, zda není lepší pracovat rovnou s max_i a i zrušit.
4) Navrhuji ti, abys přesunul podmínku max_j < j až za vnitřní cyklus. To proto, že V současné chvíli testuješ max_j proti všem pozicím elementů v řádku a to v každém cyklu opakovaně. Ale já bych řekl, tebe určitě zajímá, zda max_j neni náhodou menší než celkový počet elementů v řádku. A to budeš vědět až cyklus skončí.   

Tyhle nedostatky přímo funkčnost neovlivní, ale jednak znamenají plýtváním zdrojů mašiny (časem procesoru a pamětí) a jednak hrubě znesnadní čtení kódu v případě rozsáhlejšího souboru. No jen si představ jak by se to četlo, když by jsi měl těch vnořených cyklů několik a byly o něco delší.

5) Pokud vyloženě není tvým záměrem zjišťovat, kolik řádků a kolik elementů v nich je ve vstupu navíc (alespoň v původním zadání o tom nic nebylo), tak já osobně bych z cyklu vyskočil, jakmile bych dosáhl MAX_COLS  nebo MAX_ROWS. Tím si kód zpřehledníš a zjednodušíš, a odpadnou ti ty korekce před výpisem pole.

AHOJ A DIKY ZA VYBORNE POSTREHY

2) navratovu hodnotu fpopen() som osetril
4) max_j < j som tiez dal az za vnutorny cyklus
5) kvoli kontrole bolo mojim zamerom vypisat aj kolko riadkov a stlpcov ma vstupny subor.

Normalne by som nedeklaroval vsetky premenne na zaciatku main(), ale pomocne premenne deklaroval az na mieste kde ich idem pouzit, napr.:  for (int i = 0; ...)
Dalej by som na jednotlive casti urobil samostatne funkcie, t.j.:
- funkciu ktora nacita nazov suboru z command line argumentu programu,
- funkciu ktora nacita data zo suboru do pola
- funkciu ktora vypise obsah pola

Dalo by sa na tom este dost popracovat, ale tazatel tu nedava k tomu ziadny feedback  takze to nema vyznam.

Pripajam zmeneny zdrojak

matica.c

Kód: [Vybrat]

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_ROWS 100
#define MAX_COLS 100
#define MAX_LINE_LENGTH 256

int main()
{
    FILE *fp;
    char file_name[MAX_LINE_LENGTH] = "matrix.dat";
    char data_line[MAX_LINE_LENGTH];
    char *matrix_element;
    int i, j, max_i, max_j;
    int matrix[MAX_ROWS][MAX_COLS] = { 0 };

    if (!(fp = fopen(file_name,"r"))) {
        printf("ERROR: file \"%s\" not found !\n", file_name);
        return 1;
    }

    printf("Reading data from the file \"%s\":\n\n", file_name);
    max_j = 0;
    i = 0;
    while (fgets(data_line, MAX_LINE_LENGTH, fp) != NULL) {
        // Remove trailing \n
        data_line[strcspn(data_line, "\n")] = 0;

        // split line into matrix elements
        matrix_element = strtok(data_line, ",;\t ");
        j = 0;
        while(matrix_element != NULL) {
            printf("%s ", matrix_element);
            // populate matrix
            if ((i < MAX_ROWS) && (j < MAX_COLS)) {
                matrix[i][j] = atoi(matrix_element);
            }
            j++;
            matrix_element = strtok(NULL, ",;\t ");
        }
        if (max_j < j) max_j = j;
        printf("\n");
        i++;
    }
    max_i = i;
    fclose(fp);

    printf("\n..Done.\n\n");

    printf("Data file contains %d rows and %d columns\n\n", max_i, max_j);

    // eventually, adapt maximum indexes to the size of the matrix
    if (max_i > MAX_ROWS) {
        printf("WARNING: due to the declared size, matrix can only store\n"
               "         %d rows of a total of %d rows from the file.\n\n",
               MAX_ROWS, max_i);
        max_i = MAX_ROWS;
    }
    if (max_j > MAX_COLS) {
        printf("WARNING: due to the declared size, matrix can only store\n"
               "         %d columns of a total of %d columns from the file.\n\n",
               MAX_COLS, max_j);
        max_j = MAX_COLS;
    }

    // now print the matrix
    printf("The matrix is:\n\n");
    for (i = 0; i < max_i; i++) {
        for (j = 0; j < max_j; j++) {
            printf("%d\t", matrix[i][j]);
        }
        printf("\n");
    }

    return 0;
}


[mr.rubik]

Kod posledniho reseni jsem nestudoval do detailu, nicmene, reseni je vcelku nestastne a to z nasledujicich duvodu:

vse je tzv. nam*dane v mainu, neexistuje cleneni do funkci, ktere ten dany kus kodu "pojmenovavaji", minimalne ten print matice by mela byt funkce, i pro debugovaci ucely

jak jsem psal vyse, manualni parsovani vstupu pomoci fgets, strcspn apod. je naprosto nevhodne a to z nasledujicich duvodu:

• je to nestandardni: ostatni budou muset kontrolovat, co tim chtel basnik rici a potom se dohadovat o chybach
• scanf a getline tohle vsechno davno resi - a resi to spravne a univerzalne

muj nazor na komentare je, ze v kodu by jich melo byt co nejmene, protoze to vypovida o tom, ze autor neumi svuj kod sestavit tak, aby komentare nepotreboval (tj. mel by pouzivat standardni funkce), krasne to vystihuje komentar "// Remove trailing \n" - tohle presne resi getline a je to popsane v jeho dokumentaci

tohle je spis o definici problemu, ale i tak si nemyslim, ze je vhodne nechat program projit bez chyby pokud se prekroci limit na rozmery matice - k cemu pak je vypocet nad necelou matici? dale pak se program tvari, ze neco dela, nacita radky, parsuje, ale nic neuklada? to je dle meho nazoru podivne

tohle taky zalezi na definici a neexistuje univerzalni pravda, nicmene tyhle "skolni" priklady (stejne tak drtiva vetsina leetcode apod. problemu) parsuje vstupy ze stdin, je to i univerzalnejsi v tom, ze pokud ma clovek vstupy v souboru, muze ten soubor samozrejme presmerovat na vstup programu, ale zaroven muze zadavat male testovaci veci na stdin, odpada tim take osetrovani otevirani souboru, jmeno souboru (ktere je tady stejne hardcoded) atd.

Priznavam, ze jsem nejaky ten cas v pure Ccku nekodil, ale tohle je takova mala pekna ulozka pro oziveni, takze bych si dovolil take prispet svoji troskou do mlyna. Kdo najde nejakou zasadni chybu, ma u me pivo Automatizaci testovani ponechavam jako cviceni pro zvidaveho ctenare.

[mikrom]

..

Dik za vsetky postrehy a hinty.
To ze v tom mojom zdrojaku je vsetko spolu nam*dane v mainu som pisal v predoslom komente, ze by to bolo treba rozbit do funkcii, ale nakolko tazatel nedava feedback nedavalo mi to zmysel.
Ze ma C standardnu funkciu getline() som doteraz vobec nevedel, asi to bude nejaka novinka o ktorej neviem, lebo C nepouzivam. Urcite tu funkciu vyskusam.

VELKE DIK za prilozeny kod. Videt ze si si s tim dal hodne namahy. Este som ho neskusal, ale na prvy pohlad vyzera ze to je skor advanced level ako pre zaciatocnika. Vyskusam si ho a prestudujem.

[snugar_i]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

[Reklama]

[mr.rubik]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

Dobra otazka!

Vec se ma tak, ze to, co je const je pointer na strukturu typu Matrix, tj. nemuzete zmenit rowCount, colCount, ale hlavne, nemuzete priradit jiny pointer do ukazatele data. A to je vlastne to, co jsem tim "v tu chvili" chtel rict: az tahle funkce skonci, vsechny membery budou v puvodnim stavu, i ten pointer bude porad ukazovat tam, kam puvodne, ale ta pamet uz bude uvolnena.

Nicmene, tohle je trochu obskurita Ccka: content toho pointeru data uvnitr pointeru, ktery ukazuje na strukturu, ve ktere je obsazen uz const sam o sobe neni. Jinak by se to ani nezkompilovalo, free vyzaduje non-const pointer.

Uznavam, ze tohle je trochu divne a kdybych si delal self-review, asi bych se nad tim pozastavil. Ale spis bych to tak nechal a z pohledu konzistence zmenil ostatni mista, kde inicializuju pointery na NULL. Spis se asi priklanim k neinicializaci promennych, pokud vylozene nepotrebuju a spis nastavit "kontrakt", kdy muzu data pouzivat - jako napriklad tady: kdyz row.colCount je 0, nema cenu lezt do row.data.

Nebo jako v tomto pripade: nastavil jsem "kontrakt", ze kdyz se zavola freeMatrix, matice uz neni pouzitelna a musi si to ohlidat programator. Prijde mi to lepsi nez neustale kontrolovat jestli pointery nejsou NULL i kdyz vim, ze proste v te dane casti kodu NULL nejsou.

[vrit]

Dobry den. Chtel bych ze souboru nacist ctvercovou matici o neznamem poctu radku/sloupcu a nasledne ji opet printnout. Nemuzu prijit na to proc to nenacita zadny cisla. Kod i input je v priloze. Predem dekuji za jakykoli feedback <3

1. Radek 16: pouzivas i < COL_MAX, ale mas pouzit j
2. Radek 24: Tato podminka je nesmysl, scanf pri formatu %d neco ulozi pouze pokud se jedna o cislo v textove forme
3. Radek 36: pouzivas i<countRow ale mas pouzit j a taky mas pouzit countCols (pokud matice neni garantovane ctvercova)

Misto i,j doporucuju pouzit row,col, mozna te to rychleji trkne kdyz udelas chybu.

Jinak jsem tvuj kod vzal, opravil a udelal minimalni upravy, aby to bylo ve tvem stylu. Pridal jsem zpracovani po radcich, ale zachoval jsem formatovany vstup pomoci sscanf (obdoba scanf, ktera ale jako vstup pouziva retezec, namisto standardniho vstupu). Muzes si je porovnat.

Kód: [Vybrat]

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define ROW_MAX 100
#define COL_MAX 100
#define LINE_MAX 256

int main()
{
char line[LINE_MAX];
char *p;

int array[ROW_MAX][COL_MAX] = { 0 };
int countRows = 0;
int countCols = 0;
int matrixScan = 0;

printf("Matice:\n");

for (int i = 0; i < ROW_MAX; i++) // loads matrix numbers into array
{
if (fgets(line, LINE_MAX, stdin) == NULL)
break;

if ((p = strtok(line, " ")) == NULL)
break;

for (int j = 0; j < COL_MAX; j++)
{

matrixScan = sscanf(p, "%d", &array[i][j]);

if (matrixScan != 1)
return 1;

countCols = i > countCols ? i : countCols;

if ((p = strtok(NULL, " ")) == NULL)
break;
}

countRows++;

if (matrixScan == EOF)
break;
}

printf("Pocet radku: %d\n", countRows);

for (int i = 0; i < countRows; i++) //prints matrix
{
printf("\n");

for (int j = 0; j < countCols; j++)
{
printf("%d ", array[i][j]);
}
}

return 0;
}


[vrit]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

Vec se ma tak, ze to, co je const je pointer na strukturu typu Matrix, tj. nemuzete zmenit rowCount, colCount, ale hlavne, nemuzete priradit jiny pointer do ukazatele data. A to je vlastne to, co jsem tim "v tu chvili" chtel rict: az tahle funkce skonci, vsechny membery budou v puvodnim stavu, i ten pointer bude porad ukazovat tam, kam puvodne, ale ta pamet uz bude uvolnena.

@mr.rubik: Nesouhlasim, konkretne s tvrzenim "to, co je const je pointer na strukturu typu Matrix". Neni to pravda. Pointer neni const ale ukazuje na const data, cili pres nej nelze zapisovat.

Kód: [Vybrat]

MatrixRow *const row; // konstantni ukazatel na typ MatrixRow (nelze primo zmenit kam ukazuje)
const MatrixRow *row; // ukazatel na typ const MatrixRow (nelze pres nej zapisovat)
const MatrixRow *const row; // konstantni ukazatel na typ const MatrixRow (nelze pres nej zapisovat, ani primo zmenit kam ukazuje)


[vrit]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

Bezna implementace free() nic s daty na ktera pointer ukazuje nedela, obycejne existuje nekde v pameti seznam vsech alokovanych chunku pameti, a v tomto seznamu se odstrani zaznam o alokovane pameti na niz posilame pointer jako parametr funkce free(). Timpadem si alokator tento chunk pameti "oznaci jako znovu pouzitelny" a to je cele. Z tohoto pohledu spada dealokace do kategorie read-only.

Muze existovat nejaka bezpecnostni verze, ktera napriklad pri dealokaci vsechno prepise nulami, ale neni to bezne. Data tam zustanou dokud je nekdo jiny neprepise. Dealokaci se vsak programator zavazuje, ze s nimi jiz nebude nakladat.

[Ink]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

Bezna implementace free() nic s daty na ktera pointer ukazuje nedela, obycejne existuje nekde v pameti seznam vsech alokovanych chunku pameti, a v tomto seznamu se odstrani zaznam o alokovane pameti na niz posilame pointer jako parametr funkce free(). Timpadem si alokator tento chunk pameti "oznaci jako znovu pouzitelny" a to je cele. Z tohoto pohledu spada dealokace do kategorie read-only.

Muze existovat nejaka bezpecnostni verze, ktera napriklad pri dealokaci vsechno prepise nulami, ale neni to bezne. Data tam zustanou dokud je nekdo jiny neprepise. Dealokaci se vsak programator zavazuje, ze s nimi jiz nebude nakladat.

Nikdo rozumný ale už nemůže předpokládat, že tam ten uvolněný objekt pořád je a o to jde. Z pohledu programové logiky a ne nějaké technické nahodilosti.

[vrit]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

Bezna implementace free() nic s daty na ktera pointer ukazuje nedela, obycejne existuje nekde v pameti seznam vsech alokovanych chunku pameti, a v tomto seznamu se odstrani zaznam o alokovane pameti na niz posilame pointer jako parametr funkce free(). Timpadem si alokator tento chunk pameti "oznaci jako znovu pouzitelny" a to je cele. Z tohoto pohledu spada dealokace do kategorie read-only.

Muze existovat nejaka bezpecnostni verze, ktera napriklad pri dealokaci vsechno prepise nulami, ale neni to bezne. Data tam zustanou dokud je nekdo jiny neprepise. Dealokaci se vsak programator zavazuje, ze s nimi jiz nebude nakladat.

Nikdo rozumný ale už nemůže předpokládat, že tam ten uvolněný objekt pořád je a o to jde. Z pohledu programové logiky a ne nějaké technické nahodilosti.

Souhlas... Jak jsem jiz zminil

Dealokaci se vsak programator zavazuje, ze s nimi jiz nebude nakladat.

[mr.rubik]

@mr.rubik: Moje znalosti C jsou dost neaktuální, takže k tvýmu kódu mám hloupej dotaz - proč funkce freeMatrix a freeMatrixRow žerou const pointery? Myslel jsem, že const značí, že se s parametrem pracuje v podstatě "read-only", což dealokace jeho části úplně není? Díky za osvětlení

Vec se ma tak, ze to, co je const je pointer na strukturu typu Matrix, tj. nemuzete zmenit rowCount, colCount, ale hlavne, nemuzete priradit jiny pointer do ukazatele data. A to je vlastne to, co jsem tim "v tu chvili" chtel rict: az tahle funkce skonci, vsechny membery budou v puvodnim stavu, i ten pointer bude porad ukazovat tam, kam puvodne, ale ta pamet uz bude uvolnena.

@mr.rubik: Nesouhlasim, konkretne s tvrzenim "to, co je const je pointer na strukturu typu Matrix". Neni to pravda. Pointer neni const ale ukazuje na const data, cili pres nej nelze zapisovat.

Kód: [Vybrat]

MatrixRow *const row; // konstantni ukazatel na typ MatrixRow (nelze primo zmenit kam ukazuje)
const MatrixRow *row; // ukazatel na typ const MatrixRow (nelze pres nej zapisovat)
const MatrixRow *const row; // konstantni ukazatel na typ const MatrixRow (nelze pres nej zapisovat, ani primo zmenit kam ukazuje)


Ano, mate pravdu, spatne jsem se vyjadril. Kazdopadne si myslim, ze jsem to popsal v nasledujicim textu spravne. Nabouchal jsem to vecer a mockrat jsem to po sobe necetl. Uznavam, ze tohle je vec k diskuzi, nicmene nevidim to jako nejaky vyznamny prohresek (typu buffer ovf apod.).

V C++ jeste existuje bitwise and logical constness - jsou to dve ruzne veci. Proto napriklad existuje keyword mutable.