K & amp; Esercizio R: Il mio codice funziona, ma sembra puzzolente; Consigli per la pulizia?

StackOverflow https://stackoverflow.com/questions/161873

  •  03-07-2019
  •  | 
  •  

Domanda

Sto lavorando al libro K & amp; R. Ho letto più avanti di quanto non abbia fatto esercizi, soprattutto per mancanza di tempo. Sto recuperando e ho fatto quasi tutti gli esercizi del capitolo 1, che è il tutorial.

Il mio problema era l'esercizio 1-18. L'esercizio è di:

  

Scrivi un programma per rimuovere gli spazi vuoti finali e   schede dalla riga di input e per eliminare righe completamente vuote

Il mio codice (sotto) lo fa e funziona. Il mio problema è il metodo di trim che ho implementato. Sembra ... sbagliato ... in qualche modo. Come se vedessi un codice simile in C # in una revisione del codice, probabilmente impazzirei. (C # è una delle mie specialità.)

Qualcuno può offrire qualche consiglio per ripulirlo - con la cattura che detto consiglio deve usare solo le conoscenze del capitolo 1 di K & amp; R. (So che ci sono molti milioni di modi per ripulirlo usando l'intera libreria C; stiamo solo parlando del capitolo 1 e dello stdio.h di base qui.) Inoltre, quando dai il consiglio, puoi spiegare perché ti aiuterà? (Dopo tutto, sto cercando di imparare! E da chi è meglio imparare se non gli esperti qui?) 

#include <stdio.h>

#define MAXLINE 1000

int getline(char line[], int max);
void trim(char line[], char ret[]);

int main()
{
    char line[MAXLINE];
    char out[MAXLINE];
    int length;

    while ((length = getline(line, MAXLINE)) > 0)
    {
        trim(line, out);
        printf("%s", out);
    }

    return 0;
}

int getline(char line[], int max)
{
    int c, i;

    for (i = 0; i < max - 1 && (c = getchar()) != EOF && c != '\n'; ++i)
        line[i] = c;

    if (c == '\n')
    {
        line[i] = c;
        ++i;
    }

    line[i] = '\0'; 
    return i;
}

void trim(char line[], char ret[])
{
    int i = 0;

    while ((ret[i] = line[i]) != '\0')
        ++i;

    if (i == 1)
    {
        // Special case to remove entirely blank line
        ret[0] = '\0';
        return;
    }

    for (  ; i >= 0; --i)
    {
        if (ret[i] == ' ' || ret[i] == '\t')
            ret[i] = '\0';
        else if (ret[i] != '\0' && ret[i] != '\r' && ret[i] != '\n')
            break;
    }

    for (i = 0; i < MAXLINE; ++i)
    {
        if (ret[i] == '\n')
        {
            break;
        }
        else if (ret[i] == '\0')
        {
            ret[i] = '\n';
            ret[i + 1] = '\0';
            break;
        }
    }
}

EDIT: apprezzo tutti i suggerimenti utili che sto vedendo qui. Vorrei ricordare alla gente che sono ancora un n00b con C, e in particolare non sono ancora arrivato ai puntatori. (Ricorda il bit su Ch.1 di K & amp; R - Ch.1 non fa puntatori.) I " kinda " ottenere alcune di quelle soluzioni, ma sono ancora un tocco avanzato per dove sono ...

E la maggior parte di ciò che sto cercando è il metodo di trim stesso, in particolare il fatto che sto passando in rassegna 3 volte (che sembra così sporco). Mi sento come se fossi solo un tocco più intelligente (anche senza la conoscenza avanzata di C), questo avrebbe potuto essere più pulito.

È stato utile?

Soluzione

Non c'è motivo di avere due buffer, è possibile tagliare la linea di input in posizione 

int trim(char line[])
{
    int len = 0;
    for (len = 0; line[len] != 0; ++len)
        ;

    while (len > 0 &&
           line[len-1] == ' ' && line[len-1] == '\t' && line[len-1] == '\n')
        line[--len] = 0;

    return len;
}

Restituendo la lunghezza della linea, è possibile eliminare le linee vuote testando le linee di lunghezza diversa da zero 

if (trim(line) != 0)
    printf("%s\n", line);

EDIT: puoi rendere il ciclo while ancora più semplice, assumendo la codifica ASCII. 

while (len > 0 && line[len-1] <= ' ')
    line[--len] = 0;

Altri suggerimenti

Se stai rispettando il capitolo 1, mi sembra abbastanza buono. Ecco cosa consiglierei dal punto di vista della revisione del codice:

Quando controlli l'uguaglianza in C, metti sempre prima la costante 

if (1 == myvar)

In questo modo non farai mai accidentalmente qualcosa del genere: 

if (myvar = 1)

Non puoi cavartelo in C #, ma si compila bene in C e può essere un vero diavolo per il debug.

trim () è troppo grande.

Quello che penso sia necessario è una funzione strlen-ish (vai avanti e scrivila int stringlength (const char * s))

Quindi hai bisogno di una funzione chiamata int scanback (const char * s, const char * match, int start) che inizia all'avvio, scende a z fintanto che il carattere sottoposto a scansione sull'id contenuto nelle corrispondenze, restituisce il ultimo indice in cui viene trovata una corrispondenza.

Quindi hai bisogno di una funzione chiamata int scanfront (const char * s, const char * match) che inizia da 0 e scansiona in avanti fintanto che il carattere scansionato su s è contenuto nelle partite, restituendo l'ultimo indice in cui una corrispondenza è stato trovato.

Quindi hai bisogno di una funzione chiamata int charinstring (char c, const char * s) che restituisce diverso da zero se c è contenuto in s, 0 altrimenti.

Dovresti essere in grado di scrivere trim in questi termini.

Personalmente per mentre costruisce:

Preferisco quanto segue: 

while( (ret[i] = line[i]) )
        i++;

a: 

while ((ret[i] = line[i]) != '\0')
        ++i;

Entrambi controllano contro! = 0 ma il primo sembra un po 'più pulito. Se il carattere è diverso da 0, il corpo del ciclo verrà eseguito altrimenti verrà interrotto dal ciclo.

Anche per le dichiarazioni 'for', pur essendo sinteticamente valido, trovo che quanto segue: 

for (  ; i >= 0; --i)

mi sembra 'strano' e in effetti è una potenziale soluzione da incubo per potenziali bug. Se stavo rivedendo questo codice, sarebbe come un avvertimento rosso brillante come. In genere si desidera utilizzare i loop per ripetere un numero noto di volte, altrimenti considerare un ciclo while. (come sempre ci sono eccezioni alla regola, ma ho scoperto che questo in genere vale). Quanto sopra per la dichiarazione potrebbe diventare: 

while (i)
{
        if (ret[i] == ' ' || ret[i] == '\t')
        {
            ret[i--] = '\0';
        }
        else if (ret[i] != '\0' && ret[i] != '\r' && ret[i] != '\n')
        {
            break;
        }
}

Prima di tutto:

  

int main (void)

Conosci i parametri di main (). Non sono niente. (O argc & amp; argv, ma non credo sia il materiale del capitolo 1).

In stile, potresti provare le parentesi in stile K & amp; R. Sono molto più facili nello spazio verticale: 

void trim(char line[], char ret[])
{
    int i = 0;

    while ((ret[i] = line[i]) != '\0')
        ++i;

    if (i == 1) { // Special case to remove entirely blank line
        ret[0] = '\0';
        return;
    }

    for (; i>=0; --i) { //continue backwards from the end of the line
        if ((ret[i] == ' ') || (ret[i] == '\t')) //remove trailing whitespace
            ret[i] = '\0';

        else if ((ret[i] != '\0') && (ret[i] != '\r') && (ret[i] != '\n')) //...until we hit a word character
            break;
    }

    for (i=0; i<MAXLINE-1; ++i) { //-1 because we might need to add a character to the line
        if (ret[i] == '\n') //break on newline
            break;

        if (ret[i] == '\0') { //line doesn't have a \n -- add it
            ret[i] = '\n';
            ret[i+1] = '\0';
            break;
        }
    }
}

(Aggiunti anche commenti e corretto un bug.)

Un grosso problema è l'uso della costante MAXLINE - main () la usa esclusivamente per le variabili line e out ; trim (), che funziona solo su di essi, non ha bisogno di usare la costante. Dovresti passare le dimensioni come parametro proprio come hai fatto in getline ().

Personalmente metterei il codice in questo modo: 

ret[i] != '\0' && ret[i] != '\r' && ret[i] != '\n'

in una funzione separata (o anche una macro di definizione)

  1. trim dovrebbe effettivamente usare solo 1 buffer (come dice @Ferruccio).
  2. trim deve essere rotto, come dice @plinth
  3. trim non deve restituire alcun valore (se si desidera controllare la stringa vuota, riga di test [0] == 0)
  4. per un sapore C in più, usa i puntatori anziché gli indici

-go alla fine della linea (terminando 0; -quando non all'inizio della riga e il carattere corrente è spazio, sostituiscilo con 0. -back off un carattere 

char *findEndOfString(char *string) {
  while (*string) ++string;
  return string; // string is now pointing to the terminating 0
}

void trim(char *line) {
  char *end = findEndOfString(line);
   // note that we start at the first real character, not at terminating 0
  for (end = end-1; end >= line; end--) {
      if (isWhitespace(*end)) *end = 0;
      else return;
  }
}

Un altro esempio di fare la stessa cosa. Ha fatto qualche piccola violazione usando materiale specifico per C99. che non si troverà in K & amp; R. ha anche usato la funzione assert () che fa parte della libreria starndard, ma probabilmente non è trattata nel capitolo uno di K & amp; R. 

#include <stdbool.h> /* needed when using bool, false and true. C99 specific. */
#include <assert.h> /* needed for calling assert() */

typedef enum {
  TAB = '\t',
  BLANK = ' '
} WhiteSpace_e;

typedef enum {
  ENDOFLINE = '\n',
  ENDOFSTRING = '\0'
} EndofLine_e;

bool isWhiteSpace(
  char character
) {
  if ( (BLANK == character) || (TAB == character ) ) {
    return true;
  } else {
    return false;
  }
}

bool isEndOfLine( 
  char character
) {
 if ( (ENDOFLINE == character) || (ENDOFSTRING == character ) ) {
    return true;
  } else {
    return false;
  }
}   

/* remove blanks and tabs (i.e. whitespace) from line-string */
void removeWhiteSpace(
  char string[]
) {
  int i;
  int indexOutput;

  /* copy all non-whitespace character in sequential order from the first to the last.
    whitespace characters are not copied */
  i = 0;
  indexOutput = 0;
  while ( false == isEndOfLine( string[i] ) ) {
    if ( false == isWhiteSpace( string[i] ) ) {
      assert ( indexOutput <= i );
      string[ indexOutput ] = string[ i ];
      indexOutput++;
    }
    i++; /* proceed to next character in the input string */
  }

  assert( isEndOfLine( string[ i ] ) );
  string[ indexOutput ] = ENDOFSTRING;

}

Ecco la mia pugnalata all'esercizio senza sapere cosa c'è nel Capitolo 1 o K & amp; R. Presumo i puntatori? 

#include "stdio.h"

size_t StrLen(const char* s)
{
    // this will crash if you pass NULL
    size_t l = 0;
    const char* p = s;
    while(*p)
    {
        l++;
        ++p;
    }
    return l;
}

const char* Trim(char* s)
{
    size_t l = StrLen(s);
    if(l < 1)
        return 0;

    char* end = s + l -1;
    while(s < end && (*end == ' ' || *end == '\t'))
    {
        *end = 0;
        --end;
    }

    return s;
}

int Getline(char* out, size_t max)
{
    size_t l = 0;
    char c;
    while(c = getchar())
    {
        ++l;

        if(c == EOF) return 0;
        if(c == '\n') break;

        if(l < max-1)
        {
            out[l-1] = c;
            out[l] = 0;
        }
    }

    return l;
}

#define MAXLINE 1024

int main (int argc, char * const argv[]) 
{
    char line[MAXLINE];
    while (Getline(line, MAXLINE) > 0)
    {
        const char* trimmed = Trim(line);
        if(trimmed)
            printf("|%s|\n", trimmed);

        line[0] = 0;
    }

    return 0;
}
Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top