[C] Conferma svolgimento esercizio

[tecfil]

Ciao a tutti!

Sono sempre alle prese con i miei esercizi in C, e vorrei chiedervi una conferma su questo esercizio :-D

Si sviluppi un programma in linguaggio C che riceva in ingresso una sequenza di al più 1000 cifre binarie (1 o 0) terminate dal carattere /. Tale sequenza dovrà essere fornita in ingresso ad una funzione che dovrà fornire il numero delle cifre pari a 0 e il risultato della computazione del codice di parità dispari su tale sequenza (1 per codice corretto, 0 per codice errato). (10 punti)
Ad esempio, se in ingresso al programma viene fornita la sequenza:
10111111111111110000000000000000111111111111111111000000011/
la funzione deve computare 24 per il numero degli zeri, e 1 come risultato del controllo di parità dispari.

Ecco il codice che ho scritto:

Codice: Seleziona tutto

#include <stdio.h>
#define dim 1000
int totzeri (char codice[], int tot, int zeri);
int main ()
{
int i=0, tot=0, zeri;
char codice[dim], c;
while ((c=getchar()) != '/')
{
codice[i++]=c;
tot++;
}
printf("Risultato computazione: %d\n", totzeri(codice, tot, zeri));
return 0;
}
int totzeri (char codice[], int tot, int zeri)
{
int i, zero=0, disp=0;
for (i=0;i<tot;i++)
{
if (codice[i]%2==0)
zero++;
else (disp++);
}
printf("\n\nTotale zeri inseriti: %d\n", zero);
if (disp%2!=0)
return 1;
else
{
return 0;
}
}


Secondo voi è giusto il programma? Quando mi viene chiesto di controllare il codice di parità dispari, io devo controllare che se il numero di 1 è dispari, allora il codice è giusto e deve ritornare 1, altrimenti se è pari allora è sbagliato e deve ritornare 0.

Grazie mille
Ciaoo :)

[DirtyDeeds]

Secondo voi è giusto il programma?

Forse è giusto, ma non mi piace. Innanzitutto la formattazione lo rende di difficile lettura. Poi:

Codice: Seleziona tutto

#define dim 1000

Le define si denotano tipicamente con lettere maiuscole:

Codice: Seleziona tutto

#define DIM 1000

Codice: Seleziona tutto

int totzeri (char codice[], int tot, int zeri);

In genere, quando si dichiarano i prototipi di funzione, per vari motivi, è meglio non mettere i nomi delle variabili:

Codice: Seleziona tutto

int totzeri(char [], int, int);

Poi, a cosa dovrebbe servire il terzo argomento?

Codice: Seleziona tutto

while ((c=getchar()) != '/')
{
codice[i++]=c;
tot++;
}

In questo ciclo non c'è nessun controllo degli errori. In particolare:

1) Non viene controllato che il codice sia effettivamente binario (e il controllo non è fatto neanche nella funzione).
2) Non c'è limite al numero di caratteri in ingresso prima del '/', per cui si può avere un buffer overflow.

Inoltre, tot non è un nome significativo per quella variabile.

Codice: Seleziona tutto

printf("Risultato computazione: %d\n", totzeri(codice, tot, zeri));

Non mi è chiaro perché la scrittura dei risultati avviene in parte nel main e in parte nella funzione totzeri.

Codice: Seleziona tutto

int totzeri(char codice[], int tot, int zeri)
{
    int i, zero=0, disp=0;
    for (i=0;i<tot;i++)
    {
        if (codice[i]%2==0)
            zero++;
        else
            (disp++);
    }
    printf("\n\nTotale zeri inseriti: %d\n", zero);
    if (disp%2!=0)
        return 1;
    else
    {
        return 0;
    }
}


Il parametro zeri a cosa dovrebbe servire? Poi, se la stringa contiene solo 0 e 1, c'è un modo più semplice per contare quanti 1 ci sono Le parentesi (tonde e graffe) le hai messe un po' a caso.

La variabile del ciclo for, conviene dichiararla nel ciclo:

Codice: Seleziona tutto

for (int i = 0; i < tot; ++i)

[posta10100]

Il codice è corretto, però trovo inutile scomodare una funzione per il calcolo degli zeri.
Oltretutto in questo modo rifai due volte il lavoro: prima memorizzi tutto e poi scorri l'array per contare gli zeri.

Secondo me sarebbe più semplice (e veloce) calcolare il numero degli zeri direttamente nel main dove leggi i caratteri da tastiera.
Ogni volta che arriva un carattere lo confronti con lo '0' e se sono uguali allora incrementi il contatore.
poi alla fine devi solo vedere se il numero è pari o dispari.

Per un programmino così semplice non si nota la differenza, ma è bene abituarsi fin da subito a scrivere codice efficiente ma soprattutto semplice.



Edit: DirtyDeeds è stato più veloce di me! Però ormai il posto l'ho scritto

[DirtyDeeds]

C'è un'altra fonte di errore che mi ero scordato di segnalare prima. La funzione getchar ritorna int. Il motivo per cui ritorna int e non char è per poter accomodare il valore EOF. Ora supponi che venga raggiunta la fine del file di ingresso, EOF viene valutato come diverso da '/' e quindi le istruzioni all'interno del ciclo while vengono eseguite e poi tutto si ferma in attesa di un altro carattere che potrebbe non arrivare mai. Inoltre, l'intero EOF viene implicitamente convertito in char nella linea

Codice: Seleziona tutto

codice[i++]=c;

e nella conversione potrebbe anche generare un codice valido, equivalente a 0 o 1.

E un'ultima cosa. L'istruzione

Codice: Seleziona tutto

if (codice[i]%2==0)

cosa dovrebbe verificare? che codice[i] abbia valore 0 o 1? Se è così, non lo fa, perché codice è un char che può essere '0' o '1' ma non ha valore 0 o 1. Il valore è quello corrispondente alla codifica utilizzata dal compilatore, che potrebbe anche non essere quella ASCII.

[posta10100]

Ora supponi che venga raggiunta la fine del file di ingresso

Ma i caratteri non arrivano da tastiera?

[DirtyDeeds]

Ma i caratteri non arrivano da tastiera?

Prova a digitare Ctrl+Z

Ma non solo: e se l'utente fa una redirezione dell'input?

[posta10100]

Vero, non pensavo alle combinazioni di tasti!

[DirtyDeeds]

Prova per esempio a compilare questo programma:

Codice: Seleziona tutto

#include <stdio.h>

int main(void)
{
    char c;
    char buf[10];
    int i = 0;

    while ((c = getchar()) != '/') {
        buf[i++] = c;
    }

    buf[i] = '\0';

    printf("%s\n", buf);
}

che ha tutti i difetti di quello dell'OP. Chiamalo "prova". Poi crea un file "boh.txt" contenente solo "Ciao" (per esempio), o un'altra stringa non terminata dal carattere '/', e da riga di comando scrivi

Codice: Seleziona tutto

prova < boh.txt

Cosa succederà?

[posta10100]

Hai ragione, così cicla fino a riempire il buffer e si rompe.

Però si può riparare facilmente:

Codice: Seleziona tutto

#include <stdio.h>

#define MAX 10

int main(void)
{
    char c;
    char buf[MAX];
    int i = 0;

    while (i < (MAX - 1) && (c = getchar()) != '/') {
        buf[i++] = c;
    }

    buf[i] = '\0';

    printf("%s\n", buf);
}

[DirtyDeeds]

Non va ancora bene. Per vederlo modifica la printf così:

Codice: Seleziona tutto

printf("\"%s\"\n", buf);