K & amp; Exercice R: Mon code fonctionne, mais se sent minable; Conseils pour le nettoyage?

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

  •  03-07-2019
  •  | 
  •  

Question

Je travaille sur le livre K & amp; J'ai lu plus en avance que j'ai fait des exercices, principalement par manque de temps. Je rattrape mon retard et ai fait presque tous les exercices du chapitre 1, qui est le tutoriel.

Mon problème était l'exercice 1-18. L’exercice consiste à:

  

Ecrivez un programme pour supprimer les espaces finaux et   onglets de la ligne d'entrée et supprimer les lignes entièrement vides

Mon code (ci-dessous) le fait et fonctionne. Mon problème avec elle est la méthode de découpage que j'ai implémentée. Ça fait… mal… d'une façon ou d'une autre. Comme si je voyais un code similaire en C # dans une révision de code, je deviendrais probablement fou. (C # étant l'une de mes spécialités.)

Quelqu'un peut-il donner des conseils sur le nettoyage de cette affaire? R. (Je sais qu’il existe de nombreuses façons de résoudre ce problème en utilisant la bibliothèque complète C; nous ne parlons ici que du chapitre 1 et de stdio.h de base.) En outre, quand vous donnez cet avis, pouvez-vous expliquer pourquoi cela aidera? (Je cherche après tout à apprendre! Et qui de mieux à apprendre que les experts ici?)

#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: J'apprécie tous les conseils utiles que je vois ici. Je voudrais rappeler aux gens que je suis toujours un n00b avec C, et que je ne suis pas encore au courant. (Rappelez-vous que le point sur le ch.1 de K & R - Ch.1 ne fait pas de pointeur.) I "kinda" obtenir certaines de ces solutions, mais ils sont toujours un contact avancé pour où je suis à ...

Et la plupart de ce que je recherche, c’est la méthode de rognage elle-même - en particulier le fait que je passe en boucle à travers 3 (qui semble si sale). Je me sens comme si j’étais juste un peu plus intelligent (même sans la connaissance avancée de C), cela aurait pu être plus propre.

Était-ce utile?

La solution

Il n'y a aucune raison d'avoir deux tampons, vous pouvez ajuster la ligne d'entrée à la place

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

En renvoyant la longueur de ligne, vous pouvez éliminer les lignes vides en testant les lignes de longueur non nulle

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

EDIT: vous pouvez rendre la boucle while encore plus simple, en supposant le codage ASCII.

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

Autres conseils

Si vous vous en tenez au chapitre 1, cela me va plutôt bien. Voici ce que je recommanderais du point de vue de la révision du code:

Lors de la vérification de l'égalité en C, mettez toujours la constante en premier

if (1 == myvar)

Ainsi, vous ne ferez jamais accidentellement quelque chose comme ceci:

if (myvar = 1)

Vous ne pouvez pas vous en sortir en C #, mais cela compile bien en C et peut être un véritable diable à déboguer.

trim () est trop gros.

Ce dont vous avez besoin, à mon avis, est une fonction strlen-ish (continuez et écrivez-la int stringlength (const char * s)).

Ensuite, vous avez besoin d’une fonction appelée int scanback (const char * s, const char * correspond, int start) qui commence au début, descend à z tant que le caractère en cours de numérisation à s id contenu dans les correspondances, renvoie le dernier index où une correspondance est trouvée.

Ensuite, vous avez besoin d’une fonction appelée int scanfront (const char * s, const char * matches) qui commence à 0 et analyse en avant tant que le caractère en cours d’analyse en s est contenu dans des correspondances, renvoyant le dernier index contenant une correspondance. est trouvé.

Ensuite, vous avez besoin d’une fonction appelée int charinstring (char c, const char * s) qui renvoie non nulle si c est contenu dans s, 0 sinon.

Vous devriez être capable d'écrire des ajustements en termes de ceux-ci.

Personnellement pour les constructions while:

Je préfère ce qui suit:

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

à:

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

Ils vérifient tous les deux! = 0 mais le premier a l’air un peu plus propre. Si le caractère est différent de 0, le corps de la boucle s'exécutera sinon il sortira de la boucle.

Aussi pour les instructions 'pour', tout en étant valide du point de vue de la syntaxe, je trouve que les éléments suivants:

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

me semble "étrange" et est en effet une solution potentielle de cauchemar pour les bugs potentiels. Si je passais en revue ce code, ce serait comme un avertissement rougeoyant semblable à. Généralement, vous souhaitez utiliser des boucles for pour itérer un nombre de fois connu, sinon utilisez une boucle while. (Comme toujours, il y a des exceptions à la règle mais j'ai constaté que cela est généralement vrai). La déclaration ci-dessus pourrait devenir:

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

Tout d'abord:

  

int main (void)

Vous connaissez les paramètres de main (). Ils ne sont rien. (Ou argc & argv, mais je ne pense pas que ce soit le contenu du chapitre 1.)

Stylewise, vous pouvez essayer des supports de style K & R. Ils sont beaucoup plus faciles sur l'espace 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;
        }
    }
}

(Ajout de commentaires et correction d'un bogue.)

Un gros problème est l'utilisation de la constante MAXLINE - main () l'utilise exclusivement pour les variables line et out ; trim (), qui ne travaille que sur eux, n'a pas besoin d'utiliser la constante. Vous devriez passer la taille en tant que paramètre, comme vous l’avez fait dans getline ().

Personnellement, je mettrais un code comme celui-ci:

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

dans une fonction séparée (ou même une macro de définition)

  1. trim ne devrait en effet utiliser qu'un seul tampon (comme le dit @Ferruccio).
  2. trim doit être rompu, comme le dit @plinth
  3. trim n'a pas besoin de renvoyer de valeur (si vous voulez vérifier si une chaîne est vide, testez la ligne [0] == 0)
  4. pour une saveur C supplémentaire, utilisez des pointeurs plutôt que des index

-go va à la fin de la ligne (terminant à 0; -Pendant que vous n'êtes pas au début de la ligne et que le caractère actuel est un espace, remplacez-le par 0. -back un caractère

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 autre exemple de faire la même chose. A commis une violation mineure en utilisant des éléments spécifiques à C99 cela ne sera pas trouvé dans K & amp; R. a également utilisé la fonction assert () qui fait partie de la bibliothèque starndard, mais n’est probablement pas traitée dans le premier chapitre 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;

}

Voici mon coup de poignard à l’exercice sans savoir ce que contient le chapitre 1 ou K & amp; R. Je suppose des pointeurs?

#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;
}
Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top