Упражнение K & R: Мой код работает, но чувствует себя вонючим; Совет для очистки? - PullRequest
Новая
       34

Упражнение K & R: Мой код работает, но чувствует себя вонючим; Совет для очистки?

8 голосов
/ 02 октября 2008

Я работаю над книгой K & R. Я читал дальше, чем делал упражнения, в основном из-за нехватки времени. Я догоняю и выполнил почти все упражнения из главы 1, которая является учебным пособием.

Моим вопросом было упражнение 1-18. Упражнение заключается в:

Напишите программу для удаления конечных пробелов и вкладки из строки ввода, и удалить полностью пустые строки

Мой код (ниже) делает это и работает. Моя проблема с этим - метод отделки, который я реализовал. Это кажется ... неправильно ... как-то. Например, если бы я видел подобный код в C # в обзоре кода, я бы, наверное, сошел с ума. (C # одна из моих специальностей.)

Может кто-нибудь предложить какой-нибудь совет по очистке этого - с уловкой, которая сказала, что совет должен использовать только знания из Главы 1 K & R. (Я знаю, что есть миллионы способов очистить это, используя полную библиотеку C ; мы просто говорим здесь о главе 1 и базовом stdio.h.) Также, когда вы даете совет, можете ли вы объяснить, почему он поможет? (В конце концов, я пытаюсь учиться! А у кого лучше учиться, чем у экспертов здесь?) 

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

РЕДАКТИРОВАТЬ: Я ценю все полезные советы, которые я вижу здесь. Я хотел бы напомнить людям, что я все еще n00b с C, и, в частности, еще не дошел до указателей. (Вспомните немного о Ch.1 в K & R - в Ch.1 нет указателей.) Я "вроде" получаю некоторые из этих решений, но они все еще немного продвинуты для того, где я нахожусь ...

И большая часть того, что я ищу, - это сам метод обрезки, в частности тот факт, что я повторяю циклы 3 раз (что кажется таким грязным). Я чувствую, что если бы я был чуть более умным (даже без глубоких знаний C), это могло бы быть чище.

Ответы [ 9 ]

9 голосов
/ 02 октября 2008

Если вы придерживаетесь главы 1, это выглядит довольно хорошо для меня. Вот что я бы рекомендовал с точки зрения проверки кода:

При проверке равенства в C всегда сначала ставьте константу 

if (1 == myvar)

Таким образом, вы никогда не будете случайно делать что-то вроде этого: 

if (myvar = 1)

Вы не можете избежать неприятностей в C #, но он прекрасно компилируется в C и может быть настоящим дьяволом для отладки.

5 голосов
/ 02 октября 2008

Нет необходимости иметь два буфера, вы можете обрезать строку ввода на месте 

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

Возвращая длину строки, вы можете устранить пустые строки, протестировав строки ненулевой длины 

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

РЕДАКТИРОВАТЬ: вы можете сделать цикл while еще проще, предполагая кодировку ASCII. 

while (len > 0 && line[len-1] <= ' ')
    line[--len] = 0;
1 голос
/ 02 октября 2008

отделка () слишком велика.

Мне кажется, что вам нужна функция strlen-ish (напишите в строку длины (const char * s)).

Затем вам нужна функция с именем int scanback (const char * s, const char * match, int start), которая запускается при запуске, уменьшается до z, пока символ, сканируемый с идентификатором s, содержащимся в совпадениях, возвращает последний индекс, где найдено совпадение.

Затем вам нужна функция с именем int scanfront (const char * s, const char * match), которая начинается с 0 и сканируется вперед, пока символ, сканируемый в s, содержится в совпадениях, возвращая последний индекс, в котором совпадение найдено.

Затем вам нужна функция с именем int charinstring (char c, const char * s), которая возвращает ненулевое значение, если c содержится в s, в противном случае 0.

Вы должны быть в состоянии написать обрезку в терминах этих.

1 голос
/ 02 октября 2008

Лично для конструкций типа:

Я предпочитаю следующее: 

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

до: 

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

Они оба проверяют против! = 0, но первый выглядит немного чище. Если значение char равно 0, то тело цикла будет выполнено, иначе оно выйдет из цикла.

Кроме того, для операторов for, хотя они являются синтаксически действительными, я считаю, что следующее: 

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

только выглядит «странно» для меня и действительно является потенциальным кошмарным решением для потенциальных ошибок. Если бы я просматривал этот код, он был бы похож на красное предупреждение. Как правило, вы хотите использовать циклы for для повторения известного числа раз, в противном случае можно создать цикл while. (как всегда есть исключения из этого правила, но я обнаружил, что это в целом верно). Выше для заявления может стать: 

while (i)
{
        if (ret[i] == ' ' || ret[i] == '\t')
        {
            ret[i--] = '\0';
        }
        else if (ret[i] != '\0' && ret[i] != '\r' && ret[i] != '\n')
        {
            break;
        }
}
0 голосов
/ 02 октября 2008

Еще один пример того же. Сделал небольшое нарушение, используя материал, специфичный для C99. это не будет найдено в K & R. также использовал функцию assert (), которая является частью библиотеки starndard, но, вероятно, не описана в первой главе 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;

}
0 голосов
/ 02 октября 2008
  1. Обрезка действительно должна использовать только 1 буфер (как говорит @Ferruccio).
  2. отделка должна быть разбита, как говорит @plinth
  3. trim не должен возвращать никаких значений (если вы хотите проверить наличие пустой строки, проверьте строку [0] == 0)
  4. для дополнительного вкуса С, используйте указатели, а не индексы

- до конца строки (заканчивается 0; - в то время как не в начале строки, а текущий символ - пробел, замените его на 0. -бэк с одного символа 

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 голосов
/ 02 октября 2008

Лично я бы поставил код так: 

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

в отдельную функцию (или даже определенный макрос)

0 голосов
/ 02 октября 2008

Вот мой удар по упражнению, не зная, что в главе 1 или K & R. Я предполагаю, что указатели? 

#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 голосов
/ 02 октября 2008

Прежде всего:

int main (void)

Вы знаете параметры для main (). Они ничто. (Или argc & argv, но я не думаю, что это материал главы 1).

По стилю, вы можете попробовать скобки в стиле K & R. Они намного легче в вертикальном пространстве: 

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

(Также добавлены комментарии и исправлена ​​одна ошибка.)

Большой проблемой является использование константы MAXLINE - main () использует ее исключительно для переменных line и out ; trim (), который работает только над ними, не должен использовать константу. Вы должны передать размер (ы) в качестве параметра, как вы это делали в getline ().

...