0
\$\begingroup\$

The following is my attempt at writing a getline() function that makes it a bit easier to work with. I suppose an easier way would be to use fgets, but hopefully the following is good enough for some feedback:

#include <stdio.h>
#include <stdbool.h>
#include <ctype.h>

#define TITLE_MAX 50

void mygets(char buffer[], size_t limit)
{
    // first non-space char until newline or EOF, 
    // eating any extra chars in buffer
    // will add \0 at the end, so will get up to N-1 chars

    int c, idx=0;
    bool started=false;
    while ((c=getchar()) != EOF && c != '\n')
    {
        if (!started && isspace(c));
        if (idx < limit-1) {
            started = true;
            buffer[idx++] = c;
        }
    }
    buffer[idx] = '\0';
}

int main(void)
{
    char tbuffer[TITLE_MAX];
    while (true) 
    {
        printf("Enter the name of the film (empty line to stop)\n");
        mygets(tbuffer, TITLE_MAX);
        if (*tbuffer=='\0') break;
        printf("The title is: %s\n", tbuffer);
    }
}

Working example on onlinegdb

How does it look? How could it be improved? Additionally, how are comments usually done in C code? It is usually within the function braces? Before the function? etc.

\$\endgroup\$
6
  • \$\begingroup\$ David542, how do you want to distinguish between end-of-file and reading only "\n"? \$\endgroup\$ Commented Mar 11, 2021 at 2:37
  • \$\begingroup\$ What is the goal with code like if (!started && isspace(c));? \$\endgroup\$ Commented Mar 11, 2021 at 2:50
  • \$\begingroup\$ @chux-ReinstateMonica to skip leading whitespace (\s being used to mean space): \s\s\s Hello, chux --> Hello, chux. But it looks like that's bad code and there's an error within it. \$\endgroup\$
    – David542
    Commented Mar 11, 2021 at 2:54
  • \$\begingroup\$ David542 Why skip all leading white-space and not symmetrically all trailing white-space (aside from 1 '\n') as with " ABC \n"? \$\endgroup\$ Commented Mar 11, 2021 at 2:59
  • \$\begingroup\$ @chux-ReinstateMonica no reason, I suppose doing both would be better as you suggest. \$\endgroup\$
    – David542
    Commented Mar 11, 2021 at 3:11

2 Answers 2

2
\$\begingroup\$
  • You should be getting a warning (at least I do):

    warning: if statement has empty body [-Wempty-body]
      if (!started && isspace(c));
                                 ^
    

    This line does absolutely nothing. Judging from the comment, you probably meant

      if (!started && isspace(c))
          continue;
    
  • The loop looks overcomplicated. Its logic cries to be split into two independent actions: skip_initial_spaces and actually_read_data, performed sequentially (again, assuming that I read the comment correctly).

    Factoring them out into the functions of their own will also make your code comply with a single responsibility principle.

  • I strongly advise against void functions. Do not discard the information you have already computed; it is very likely the caller will need it. In this case, returning idx will spare the caller an additional call to strlen.

EDIT. On splitting up the functionality, something along these lines:

int skip_leading_whitespaces()
{
    int c;
    while (((c = getchar()) != EOF) && isspace(c)) {
    }
    return c;
}

int actually_read_data(int c, char buffer[], size_t limit)
{
    int idx = 0;
    while ((c != EOF) && (c != '\n')) {
        buffer[idx++] = c;
        c = getchar();
    }
    buffer[idx] = 0;
    return idx;
}

int mygets(char buffer[], size_t limit)
{   
    int c = skip_leading_whitespaces();
    return actually_read_data(c, buffer[], limit);
}
\$\endgroup\$
4
  • \$\begingroup\$ thanks for this. Could you describe how it might be split up into skip_initial_spaces and actually_read_data in a couple lines of code or so? \$\endgroup\$
    – David542
    Commented Mar 9, 2021 at 1:57
  • \$\begingroup\$ @David542 see edit. \$\endgroup\$
    – vnp
    Commented Mar 9, 2021 at 2:10
  • \$\begingroup\$ OP's code only reads up to 1 line. This code may read multiple lines with skip_leading_whitespaces(). \$\endgroup\$ Commented Mar 11, 2021 at 2:43
  • \$\begingroup\$ actually_read_data() does not use limit. As you say " something along these lines:". \$\endgroup\$ Commented Mar 11, 2021 at 2:45
0
\$\begingroup\$

Incorrect code to skip leading white space

Asymmetrically attempts to skip leading white-spaces but not all trailing ones

Mixed types

Rather than size_t limit, int idx. Use the same type. Recommend size_t

Edge cases behavior woe

idx < limit-1 is like idx < SIZE_MAX when limit == 0. Better as idx + 1 < limit.

No return

No clear what to distinguish end-of-file from reading "\n".


I'd set aside the skip leading white-space goal for now.

\$\endgroup\$
2
  • \$\begingroup\$ thanks. Would you want to post an updated version of the code with how it can be improved? \$\endgroup\$
    – David542
    Commented Mar 11, 2021 at 4:13
  • 1
    \$\begingroup\$ @David542 Still waiting on question. \$\endgroup\$ Commented Mar 11, 2021 at 4:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.