2008-10-02 14 views
8

Estoy trabajando en el libro K & R. He leído más adelante que los ejercicios, principalmente por falta de tiempo. Me pongo al día y he hecho casi todos los ejercicios del capítulo 1, que es el tutorial.Ejercicio K & R: Mi código funciona, pero se siente mal olor; Consejos para la limpieza?

Mi problema fue el ejercicio 1-18. El ejercicio consiste en:

escribir un programa para eliminar los blancos de cola y pestañas de línea de entrada, y para eliminar por completo las líneas en blanco

Mi código (abajo) hace eso, y trabaja. Mi problema con esto es el método de recorte que implementé. Se siente ... mal ... de alguna manera. Al igual que si vi un código similar en C# en una revisión del código, probablemente me volvería loco. (C# es una de mis especialidades)

¿Alguien puede ofrecer algún consejo sobre cómo limpiar esto? Con la excusa de que dicho consejo tiene que usar únicamente el conocimiento del Capítulo 1 de K & R. (Sé que hay un trillón Maneras de limpiar esto usando la biblioteca completa de C, solo estamos hablando del Capítulo 1 y la stdio.h básica aquí. También, al dar el consejo, ¿puedes explicar por qué te ayudará? (Estoy, después de todo, tratando de aprender Y quién mejor para aprender de que los expertos aquí!?)

#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: Agradezco todos los consejos útiles que estoy viendo aquí. Me gustaría recordar a la gente que todavía soy un n00b con C, y específicamente aún no he llegado a los punteros. (Recuerde la información sobre el capítulo 1 de K & R - Ch.1 no hace punteros.) "Recibo un poco" algunas de esas soluciones, pero todavía están un poco avanzadas para saber dónde estoy. .

Y la mayor parte de lo que estoy buscando es el propio método de ajuste, específicamente el hecho de que estoy recorriendo veces (lo cual se siente tan sucio). Siento que si fuera solo un toque más inteligente (incluso sin el conocimiento avanzado de C), esto podría haber sido más limpio.

+0

, no encontramos un problema aquí ... – Ilya

+0

Obviamente, es casi 7 años tarde, por lo que no necesariamente para usted, sino para los que siguen. En este caso, K & R podría interpretarse como líneas que contienen solo caracteres '' ''cuando dicen" eliminar líneas completamente en blanco " "dado que el ciclo while en la función principal saldrá si' getline' devuelve una longitud de cero (es decir, una línea que literalmente carece de caracteres). Referencia: la lista del código en la página 29. – chucksmash

Respuesta

5

No hay ninguna razón para tener dos memorias intermedias, puede recortar la línea de entrada en lugar

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; 
} 

Al devolver la longitud de la línea, puede eliminar líneas en blanco mediante pruebas de longitud líneas diferentes a cero

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

EDITAR: Puede hacer que el bucle while sea aún más simple, asumiendo la codificación ASCII.

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

Este es el tipo de idea que estoy buscando ...Pero lo conecté y jugué con él un poco, y en lugar de recortar los espacios de los extremos y las líneas en blanco, en realidad está insertando líneas en blanco adicionales. :) –

+0

Eso es lo que sucede cuando tecleas el código a primera hora de la mañana sin comprobarlo :-) – Ferruccio

+0

¡Escribí! = En lugar de == en el ciclo while original. – Ferruccio

9

Si te apegas al capítulo 1, eso se ve bastante bien para mí. Esto es lo que recomendaría a partir de un código de revisión de punto de vista:

Al comprobar la igualdad en C, siempre ponen la primera constante de

if (1 == myvar) 

De esa manera usted nunca accidentalmente hacer algo como esto:

if (myvar = 1) 

No puede salirse con la suya en C#, pero compila bien en C y puede ser un verdadero demonio para depurar.

+0

Whoa, muchos votos a favor ... todavía no han bebido mucho, ¿qué pasa con eso? –

+0

no hay razón para desestimar consejos perfectamente válidos, personalmente no me molesto en sembrar pero de todas maneras es perfectamente válido votar. – Ilya

+1

Supongo que a algunas personas no les gusta la sintaxis (1 == x). Soy uno de ellos, pero no creo que valga la pena un voto negativo. – aib

1

Personalmente para mientras que las estructuras:

prefiero el siguiente:

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

a:

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

Ambos cheque contra = 0, pero el primero se ve un poco limpiador. Si el char es cualquier cosa que sea 0, entonces el cuerpo del bucle se ejecutará, de lo contrario saldrá del bucle.

También para 'por' declaraciones, siendo a la vez syntatically válida, considero que el siguiente:

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

sólo se ve 'extraño' a mí y de hecho es una solución potencial pesadilla para los errores potenciales. Si estuviera revisando este código, sería como una advertencia roja brillante como. Por lo general, desea utilizar ciclos for para iterar un número conocido de veces, de lo contrario, se considera un ciclo while. (como siempre hay excepciones a la regla pero he encontrado que esto generalmente es cierto). Lo anterior para la declaración podría llegar a ser:

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

Erm, quiere decir "while (ret [i] = línea [i])" .. – aib

+0

Gracias por el consejo sobre cómo reemplazar el bucle for con una construcción while. –

0

En primer lugar:

int main (void)

Usted conoce los parámetros a main(). Ellos no son nada. (O argc & argv, pero no creo que sea material del Capítulo 1)

De manera estilística, es posible que desee probar K & soportes de estilo R. Son mucho más fácil en el espacio vertical:

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; 
     } 
    } 
} 

(comentarios También se ha añadido y se fijan una bichos.)

Un gran problema es el uso de la constante MAXLINE - main() utiliza exclusivamente para la línea y variables; trim(), que solo está trabajando en ellos, no necesita usar la constante. Debería pasar el tamaño (s) como un parámetro como lo hizo en getline().

1

trim() es demasiado grande.

Lo que creo que necesita es una función strlen-ish (seguir adelante y escribirla int stringlength (const char * s)).

Luego necesita una función llamada int scanback (const char * s, const char * coincidencias, int start) que comienza al inicio, baja a z siempre que el carácter que se está escaneando en s id contenga coincidencias, regrese el último índice donde se encuentra una coincidencia.

Luego necesita una función llamada int scanfront (const char * s, const char * matches) que comienza en 0 y rastrea hacia adelante mientras el carácter que se escanea en s esté contenido en coincidencias, devolviendo el último índice donde se encuentra el partido.

Luego necesita una función llamada int charinstring (char c, const char * s) que devuelve un valor distinto de cero si c está en s, 0 en caso contrario.

Debería poder escribir ajustes en estos términos.

0

Aquí está mi puñalada en el ejercicio sin saber lo que está en el Capítulo 1 o K & R. Supongo que los punteros?

#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; 
} 
+0

eh, eso parece peligroso. ¿Qué sucede si alguien llama a Trim (""); Vas a leer la memoria que está fuera de la cadena. Y con un poco de mala suerte, también escribirás en esa memoria. – quinmars

+0

Puede haber errores en ese código. No lo probé súper a fondo. Tienes razón. La condición while loop en Trim() también debería probar que el extremo es mayor que s. Suponiendo que las cadenas crecen en las direcciones de memoria. – orj

0

Personalmente me gustaría poner un código como éste:

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

en una función separada (o incluso una definición de macro)

0
  1. ajuste de hecho debe utilizar solamente 1 búfer (como @ Ferruccio dice).
  2. ajuste necesita ser roto, como se dice @plinth
  3. recorte no necesita devuelve ningún valor (si desea comprobar si una cadena vacía, la línea de prueba [0] == 0)
  4. para el sabor extra C, punteros de uso en lugar de índices

-ir al final de la línea (terminación 0; -mientras no al comienzo de la línea y el carácter actual es el espacio, reemplazarlo con 0. -back fuera con un carácter

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; 
    } 
} 
0

Otro ejemplo de hacer lo mismo. Hizo una pequeña infracción al usar material específico de C99. que no se encontrará en K & R. También utiliza la aserción() función que es parte de la biblioteca starndard, pero probablemente no esté prevista en el capítulo uno de K & 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; 

} 
Cuestiones relacionadas