K&R Exercício: Meu código funciona, mas parece fedorento; Conselhos para limpeza?

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

  •  03-07-2019
  •  | 
  •  

Pergunta

Estou trabalhando no livro da K&R. Eu li mais à frente do que fiz exercícios, principalmente por falta de tempo. Estou alcançando e fiz quase todos os exercícios do capítulo 1, que é o tutorial.

Meu problema foi o exercício 1-18. O exercício é:

Escreva um programa para remover espaços em branco e guias à direita da linha de entrada e para excluir linhas totalmente em branco

Meu código (abaixo) faz isso e funciona. Meu problema é o método de acabamento que implementei. Parece ... errado ... de alguma forma. Como se eu visse código semelhante em C# em uma revisão de código, provavelmente ficaria louco. (C# sendo uma das minhas especialidades.)

Alguém pode oferecer alguns conselhos sobre como limpar isso - com a captura que dizia que o conselho precisa usar apenas o conhecimento do capítulo 1 de K & R. (eu sei que há um zilhão de maneiras de limpar isso usando a biblioteca C completa; nós '' estou apenas falando do capítulo 1 e do stdio.h básico aqui.) Além disso, ao dar o conselho, você pode explicar por que isso ajudará? (Afinal, estou tentando aprender! E quem é melhor aprender do que os especialistas aqui?)

#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: Agradeço todas as dicas úteis que estou vendo aqui. Gostaria de lembrar às pessoas que ainda sou um N00B com C e, especificamente, ainda não cheguei a ponteiros. (Lembre -se de que o Ch.1 de K&R - Ch.1 não faz ponteiros.) Eu "meio", recebo algumas dessas soluções, mas eles ainda são um toque avançado para onde estou ...

E a maior parte do que estou procurando é o próprio método de acabamento - especificamente o fato de que estou indo 3 tempos (que parecem tão sujos). Sinto que, se eu fosse apenas um toque mais inteligente (mesmo sem o conhecimento avançado de C), isso poderia ter sido mais limpo.

Foi útil?

Solução

Não há razão para ter dois buffers, você pode aparar a linha de entrada no 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;
}

Ao retornar o comprimento da linha, você pode eliminar linhas em branco testando linhas de comprimento diferente de zero

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

EDIT: Você pode tornar o loop enquanto o loop ainda mais, assumindo a codificação ASCII.

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

Outras dicas

Se você está mantendo o capítulo 1, isso parece muito bom para mim. Aqui está o que eu recomendaria do ponto de vista de revisão de código:

Ao verificar a igualdade em C, sempre coloque a constante primeiro

if (1 == myvar)

Dessa forma, você nunca fará algo assim:

if (myvar = 1)

Você não pode se safar disso em C#, mas isso compila bem em C e pode ser um verdadeiro diabo para depurar.

Trim () é muito grande.

O que eu acho que você precisa é de uma função strlen-ish (vá em frente e escreva-o int stringLength (const char *s)).

Então você precisa de uma função chamada int scanback (const char *s, const char *corresponde, int start) que começa no início, desce para z enquanto o personagem que está sendo digitalizado no S ID contido nas partidas, retorne o último índice onde Uma partida é encontrada.

Em seguida, você precisa de uma função chamada int scanfront (const char *s, const char *corresponde) que começa em 0 e digitaliza a frente enquanto o caractere que está sendo digitalizado em s estiver contido nas partidas, retornando o último índice onde uma correspondência é encontrada.

Em seguida, você precisa de uma função chamada int charinstring (char c, const char *s) que retorna diferentes de zero se c estiver contido em s, 0 caso contrário.

Você deve ser capaz de escrever acabamentos em termos desses.

Pessoalmente, para construções:

Eu prefiro o seguinte:

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

para:

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

Ambos verificam! = 0, mas o primeiro parece um pouco mais limpo. Se o char for algo outro Thah 0, o corpo do loop será executado, ele sairá do loop.

Também para declarações 'para', embora seja sintaticamente válido, acho que o seguinte:

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

Apenas parece 'estranho' para mim e, de fato, é uma solução potencial de pesadelo para possíveis bugs. Se eu estivesse revisando esse código, seria como um aviso vermelho brilhante. Normalmente, você deseja usar o Loops para iterar um número conhecido de vezes, caso contrário, o Cosider um loop. (Como sempre, há exceções à regra, mas eu descobri que isso geralmente é verdadeiro). O exposto acima para a declaração pode se tornar:

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

Em primeiro lugar:

int main (vazio)

Você conhece os parâmetros para Main (). Eles não são nada. (Ou argc & argv, mas não acho que seja o material 1.)

Stylewise, você pode querer experimentar suportes no estilo K & R. Eles são muito mais fáceis no espaço 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;
        }
    }
}

(Também adicionou comentários e corrigiu um bug.)

Uma grande questão é o uso da constante maxline - main () usa exclusivamente para o linha e Fora variáveis; Trim (), que está trabalhando apenas neles não precisa usar a constante. Você deve passar o (s) tamanho (s) como um parâmetro, como fez em getLine ().

Pessoalmente, eu colocaria código assim:

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

em uma função separada (ou mesmo uma macro definida)

  1. O TRIM deve realmente usar apenas 1 buffer (como @ferruccio diz).
  2. TRIM precisa ser quebrado, como @plinth diz
  3. TRIM não precisa retornar nenhum valor (se você quiser verificar se há string vazia, linha de teste [0] == 0)
  4. Para um sabor C extra, use ponteiros em vez de índices

-Port -to End of Line (terminando 0; -Quando que não no início da linha e o caractere atual é espaço, substitua -o por 0. -Back Off One char

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

Outro exemplo de fazer a mesma coisa. Fez uma pequena violação usando coisas específicas de C99. Isso não será encontrado em K&R. Também usou a função Assert () que faz parte da Biblioteca Starndard, mas provavelmente não é abordada no capítulo um 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;

}

Aqui está minha facada no exercício sem saber o que está no capítulo 1 ou na K & R., presumo que os ponteiros?

#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;
}
Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top