Function to return last line's length of a string

Question

I am completely re-writing the textbox GUI widget. In order to set the position cursor after the last character in the last line of a text, it is good if you know the exact column of it. So I created a small function that calculates this. It is meant to work sonicly.

int get_last_line_length (char *string)
{
 const int len = strlen(string) - 1;
 int i;
 for(i = len; i != -1 && string[i] != '\n'; i--);
 return (len - i);
}

• SirPython: I'm a little confused; if you needed to put the cursor in the character space at the end of the line, wouldn't you just find the length of the line and add one to it? (strlen(line) + 1).
• Edenia: Don't be confused. As I said, I am re-writing it from scratch. Which means I have to draw the cursor itself, animate it and find the exact x/y canvas coordinates to draw it to. strlen(text) is the iterator and get_last_line_length(text) is last column.

Answer 1

It's good that you've used strlen, but there's actually another standard function that could be useful here. Instead of your reverse loop, you can use strrchr to find a pointer to the last occurrence of a character (or NULL if said character is not found). As discussed in the comments, this could be (and likely is) slower in the case when a newline is not found, but unless this function is called very often or is in a very performance critical path, I can't imagine it will be an issue. If you do need to optimize it to your original implementation, I would pull the reverse loop logic out into a function.

Anyway, a bit of a review:

• Your string parameter should be a pointer to const since it's not modified.
• Personal style thing: I'm not a fan of return (...). If it matters, it also tends to be pretty rare style wise in C.
• You should probably explicitly document what happens if a line break is not found in the text. I wouldn't be sure whether strlen(string) or -1 should be returned.

• Edenia: It is designed to return the string's len if line break wasn't found.
• Edenia: Thanks. I am not sure if using strrchr on that way will perform less operations than my inverse loop..(in my biased opinion it will be only slower) but one thing is certain - I don't know what would it benefit anyway.
• Corbin: @Edenia it would indeed be slower in the case that a new line isn't found. Unless this is in a tight loop though, I doubt it will be noticeable and the clarity will likely be worth any tiny hit (unless it ends up being a hot spot).
• Edenia: I know I look overexacting. But I work into a SDL-based engine restricted to FPS. There any operation can be noticed by the FPS drop. Yes it is runtime coding using the "EiC" interpreter.

Answer 2

If len is const, then char *string should be const as well. Actually, nobody really cares if the local variable is const; it's more important that the parameter be const.

I'm not a big fan of len = strlen(string) - 1, since the -1 makes it not the length of the string.

Consider working forward instead of backwards. I particularly like strlen(last_line), which makes this code more self-explanatory.

int get_last_line_length(const char *string)
{
 const char *newline;
 const char *last_line = string;
 while ((newline = strchr(last_line, '\n'))) {
 last_line = newline + 1;
 }
 return strlen(last_line);
}

That said, I suspect that your implementation might be faster.

• Edenia: I was about to Add Comment "I suspect that my implementation might be still faster though" ..and then I saw the last line of your answer and I was like how did my unposted comment got there.
• 200_success: I'd say that your original code is good enough, especially if you are aiming for speed.
• Edenia: In that case I would aim for speed, because it slows down everything aside the text box.. and the implemented base drawing functions are just impossibly slow. Anyway, your approach is nice and I would use it in a regular circumstance.

Answer 3

As 200_success said, the len variable that's not really the length is not intuitive. It would be good to either rename the variable to last_pos, or to change the code.

A minor and possibly subjective thing, but instead of != -1 in the loop condition, >= 0 seems slightly more intuitive, hinting a descending counter, and aiding readability a little bit.

Lastly, also a minor thing, but a for loop without a body is sometimes frowned upon. If the main purpose of the loop is to count, not something else (the loop body), then it's better to make the counting operation more prominent by converting the for loop to a while loop.

int get_last_line_length (char *string)
{
 const int len = strlen(string);
 int i = len - 1;
 while (i >= 0 && string[i] != '\n') {
 --i;
 }
 return len - i - 1;
}

• 200_success: If you want to avoid having an empty for loop, I suggest for (i = len - 1; i >= 0; i--) { if (string[i] == '\n') break; }. That way, the loop is still obviously a countdown for i.
• janos: Not a bad idea. I had another reason though I didn't explain: when the counter variable is needed outside, I prefer a while loop

Answer 4

I would have liked to see some unit tests for this function, especially as there's no usage guide in comments.

I created a simple test of my own:

#include <stdio.h>
/* return 0 for a passing test, non-zero for failure */
int test_glll(const char *s, const int expected)
{
 const int actual = get_last_line_length(s);
 if (actual == expected) { return 0; }
 fprintf(stderr, "\"%s\" returned %d; expected %d\n",
 s, actual, expected);
 return 1;
}
int main(void)
{
 return test_glll("", 0)
 | test_glll("a", 1)
 | test_glll("abc", 3)
 | test_glll("\n", 0)
 | test_glll("abc\n", 0)
 | test_glll("abc\nxy", 2)
 | test_glll("abc\nnmo\nwxyz", 4);
}

In order to get this to compile, I needed to include <string.h> and to change the argument type to const char*. I still get a compilation warning about the risky conversion from size_t to int here:

const int len = strlen(string) - 1;

That cascades to considering the return type of the function - if it's to work with arbitrary strings, then we'll need to return a size_t.

---

Looking at the implementation, in the worst case we make two passes over the string contents: one in strlen() and then one in the for loop. However, we don't actually need to measure the length before we start - just make a single pass over the whole string, keeping a note of the most recently-seen newline (or, more conveniently, the immediate next index).

#include <stdlib.h>
size_t get_last_line_length(const char *s)
{
 size_t line_start = 0;
 size_t i = 0;
 while (s[i]) {
 if (s[i++] == '\n') {
 line_start = i;
 }
 }
 return i - line_start;
}

#include <stdio.h>
/* return 0 for a passing test, non-zero for failure */
int test_glll(const char *s, const size_t expected)
{
 const size_t actual = get_last_line_length(s);
 if (actual == expected) { return 0; }
 fprintf(stderr, "\"%s\" returned %zu; expected %zu\n",
 s, actual, expected);
 return 1;
}
int main(void)
{
 return test_glll("", 0)
 | test_glll("a", 1)
 | test_glll("abc", 3)
 | test_glll("\n", 0)
 | test_glll("abc\n", 0)
 | test_glll("abc\nxy", 2)
 | test_glll("abc\nnmo\nwxyz", 4);
}