Hi all,

So I'm trying to do this checksum problem for my C programming class but im running into some difficulty. My output is supposed to be this:

Line: This is short.
Checksum: 2
Line: The quick brown fox jumped over the lazy dog.
Checksum: =
Line: A.
Checksum: O
Line: B.
Checksum: P
Line: C.
Checksum: Q
Line: .

But I'm getting this:

Line: This is short.
Checksum:
Line: The quick brown fox jumped over the lazy dog.
Checksum: !
Line: A.
Checksum: /
Line: B.
Checksum: 0
Line: C.
Checksum: 1
Line: .
Checksum: .

This is my code

#include <stdio.h>

int main ()
{
while(1)
{
       char str[50] = {0};
       int num = 0;
       char final_val;

       printf("Line: "); // Request Line
       scanf("%s", str); // Read in Line

       int i;
        for (i = 0; i<=49; i++)
       {
               num+=str[i];
       }

       num = num%64;
       final_val = num;
       num = 0;
       printf("\nChecksum: %c", final_val);
       printf("\n");

        return(0);
}

The program is supposed to accept single-line messages ending with a period and displays the checksum character for each message. It should continue displaying checksums until the user enters a line with only a period. I am really stumped on this... can anyone help me out?

jsphb9000>

scanf("%s", str); // Read in Line

That doesn't read a line but rather a word since it will stop as soon as it encounters a space.

jsphb9000>

while(1)

What does stop this loop? int i; must be declared in the beginning of the block.


jsphb9000>

for (i = 0; i<=49; i++)
{
    num+=str[i];
}

Although the capacity of str is of 49 chars + 1 for '\0', it is most likely that the exact sentence will never be entered. The logic of that for loop is incorrect.

Edited 6 Years Ago by Aia: n/a

jsphb9000>

scanf("%s", str); // Read in Line

That doesn't read a line but rather a word since it will stop as soon as it encounters a space.

jsphb9000>

while(1)

What does stop this loop? int i; must be declared in the beginning of the block.


jsphb9000>

for (i = 0; i<=49; i++)
{
    num+=str[i];
}

Although the capacity of str is of 49 chars + 1 for '\0', it is most likely that the exact sentence will never be entered. The logic of that for loop is incorrect.

So how should i fix these?

To fix the first one should I replace scanf("%s", str); with fgets(str);

How do i stop the while loop and fix the logic of the for loop?

jsphb9000> To fix the first one should I replace scanf("%s", str); with fgets(str);

Yes. The proper syntax would be fgets(str, sizeof str, stdin); Where str is the variable to hold the string, sizeof str, would be the capacity (fgets will automatically accommodate the '\0' at the end of the string and will reduce the total capacity by one if necessary to do so ), stdin is the standard input.

The only side-effect in this case is that the ENTER key or '\n' will be included unless that the buffer is used in its fullness, and you need to deal with it somehow.


jsphb9000> How do i stop the while loop and fix the logic of the for loop?

line is a '\0'
always
    input line
    if line is not a period
         process line
         display result
    else
         break always loop
for (i = 0; str[i] != '\0'; i++)

Edited 6 Years Ago by Aia: n/a

Since you are reading from the stdin, you should use the function gets(char *str) which reads from the stdin and does not include the new line character (\n) in buffer

Comments
make me come out of retirement just to downvote your sorry ass
gets?? dude, are you serious???
What's next? fflush(stdin)? Forget that gets() exists.

Since you are reading from the stdin, you should use the function gets(char *str) which reads from the stdin and does not include the new line character (\n) in buffer

Never ever ever use gets() . Here's why. While you're at it, see this too.

you should use the function gets(char *str) which reads from the stdin and does not include the new line character (\n) in buffer

and you've got 88 posts here? seriously, guy, stop posting for a while.

Walt meant to post this as the reason why: http://www.gidnetwork.com/b-56.html.

there are a 100 more explanations just like that, including the Linux man pages. I believe the US and other major world governments have even told their people to not use that function.

if you don't like the newline being included in fgets, then just remove it.

if (buffer[strlen(buffer) - 1] == '\n')
    buffer[strlen(buffer) - 1] = '\0'
Comments
Oops... Thanks Jose. Are you an astronaut? :)

jsphb9000> To fix the first one should I replace scanf("%s", str); with fgets(str);

Yes. The proper syntax would be fgets(str, sizeof str, stdin); Where str is the variable to hold the string, sizeof str, would be the capacity (fgets will automatically accommodate the '\0' at the end of the string and will reduce the total capacity by one if necessary to do so ), stdin is the standard input.

The only side-effect in this case is that the ENTER key or '\n' will be included unless that the buffer is used in its fullness, and you need to deal with it somehow.


jsphb9000> How do i stop the while loop and fix the logic of the for loop?

line is a '\0'
always
    input line
    if line is not a period
         process line
         display result
    else
         break always loop
for (i = 0; str[i] != '\0'; i++)

OK so i managed to get the line is a \0 part but im still having trouble getting it to stop when it meets only a period.

#include <stdio.h>

int
main ()
{
        const int MAX_LEN = 256;
        char line[ MAX_LEN + 1];
        char dot[2] = ".";
        line[ MAX_LEN ] = 0;
        line[0] = 0;
while(1)
{
        char str[50] = {0};
        int num = 0;
        char final_val;

        printf("Line: "); // Request Line
        fgets(str, 100 , stdin); // Read in Line

        int i;
        for (i = 0; str[i] != '\0'; i++)
       {
               num+=str[i];
       }

       num = num%64;
       final_val = num;
       num = 0;
       printf("\nChecksum: %c", final_val);
       printf("\n");

        return(0);
}
}

if you don't like the newline being included in fgets, then just remove it.

if (buffer[strlen(buffer) - 1] == '\n')
    buffer[strlen(buffer) - 1] = '\0'

Close. There's no point in calling strlen twice, especially since strlen has guaranteed O(N) complexity (barring clever tricks in the implementation). It's just as easy to cache the result:

{
    size_t end = strlen(buffer) - 1;

    if (buffer[end] == '\n')
    {
        buffer[end] = '\0';
    }
}

strchr is a common solution as well:

{
    char *newline = strchr(buffer, '\n');

    if (newline != NULL)
    {
        *newline = '\0';
    }
}

If you're a fan of one-liners, strcspn can make you look like you have far too much time on your hands to come up with clever tricks:

buffer[strcspn(buffer, "\n")] = '\0';

im still having trouble getting it to stop when it meets only a period.

Well, after fgets you loop through the entire string. Perhaps you should stop on either '\0' or '.':

for (i = 0; str[i] != '\0' && str[i] != '.'; i++)

OK so i managed to get the line is a \0 part but im still having trouble getting it to stop when it meets only a period.

const int MAX_LEN = 256;
        char line[ MAX_LEN + 1];
        char dot[2] = ".";
        line[ MAX_LEN ] = 0;
        line[0] = 0;

There is not purpose for all that at this time.

while(1)
{
        char str[50] = {0};
        int num = 0;
        char final_val;

        printf("Line: "); // Request Line
        fgets(str, 100 , stdin); // Read in Line

If you want to have str[50] to be created and destroyed every time through the loop, placing it inside the while block will do that, now if it is OK to preserve it and save the process of creating it multiple times, place it outside the loop.

char str[50] = {0};
.
.
.
fgets(str, 100 , stdin); // Read in Line

Capacity of str is 50 bytes, you are telling it that it is OK to stuff it with 100 bytes if it might be.
Perhaps,you missed my previous explanation:

The proper syntax would be fgets(str, sizeof str, stdin); Where str is the variable to hold the string, sizeof str, would be the capacity (fgets will automatically accommodate the '\0' at the end of the string and will reduce the total capacity by one if necessary to do so ), stdin is the standard input.

printf("Line: "); // Request Line

To guarantee that it will be displayed in a timely manner, it must be flushed to stdout.

printf("Line: ");
fflush(stdout);

The specifications you wrote says:

The program is supposed to accept single-line messages ending with a period and displays the checksum character for each message. It should continue displaying checksums until the user enters a line with only a period.

Therefore, there's the assumption that the period will be part of the single-line and the only instance were it will be not is when it is input to stop.

Thus

Well, after fgets you loop through the entire string. Perhaps you should stop on either '\0' or '.':

for (i = 0; str[i] != '\0' && str[i] != '.'; i++)

might not work as intended in this case, since the period will be excluded from successful single-line read.

if you don't like the newline being included in fgets, then just remove it.

if (buffer[strlen(buffer) - 1] == '\n')
    buffer[strlen(buffer) - 1] = '\0'

In this case it might not be necessary to remove it at all.

#include <stdio.h>

int main (void) /* notice the explicit void and int in one line */
{
    char str[50] = {0};

    for (;;) /* for loop is better in this case, in my view, take or leave it! */
    {
        int num = 0;
        int i;      /* i gets moved to beginning of block as per C89 standard */
        char final_val;

        printf("Line: "); 
        fflush(stdout); /* ensures proper display of "Line: " */

        /*
         * reads from stdin up to but not more that the size that
         * string was set for it.
         * if the first char is a '.' then user is finished
         */
        if (fgets(str, sizeof str, stdin) != NULL && str[0] != '.') {
            /*
             * go through buffer, if the newline or the string terminator is
             * found stop. Newline first since the probability of finding it
             * first is greater.
             */

            for (i = 0; str[i] != '\n' && str[i] != '\0'; i++)
            {
                /* do calculation here */
            }
            /* display results here */
            
        } else {
            break;
        }
    }

    return(0);
}

All that follows the model I previously posted.

jsphb9000> How do i stop the while loop and fix the logic of the for loop?

line is a '\0'
always
    input line
    if line is not a period
         process line
         display result
    else
         break always loop

Edited 6 Years Ago by Aia: n/a

const int MAX_LEN = 256;
        char line[ MAX_LEN + 1];
        char dot[2] = ".";
        line[ MAX_LEN ] = 0;
        line[0] = 0;

There is not purpose for all that at this time.

while(1)
{
        char str[50] = {0};
        int num = 0;
        char final_val;

        printf("Line: "); // Request Line
        fgets(str, 100 , stdin); // Read in Line

If you want to have str[50] to be created and destroyed every time through the loop, placing it inside the while block will do that, now if it is OK to preserve it and save the process of creating it multiple times, place it outside the loop.

char str[50] = {0};
.
.
.
fgets(str, 100 , stdin); // Read in Line

Capacity of str is 50 bytes, you are telling it that it is OK to stuff it with 100 bytes if it might be.
Perhaps,you missed my previous explanation:

printf("Line: "); // Request Line

To guarantee that it will be displayed in a timely manner, it must be flushed to stdout.

printf("Line: ");
fflush(stdout);

The specifications you wrote says:

Therefore, there's the assumption that the period will be part of the single-line and the only instance were it will be not is when it is input to stop.

Thus might not work as intended in this case, since the period will be excluded from successful single-line read.


In this case it might not be necessary to remove it at all.

#include <stdio.h>

int main (void) /* notice the explicit void and int in one line */
{
    char str[50] = {0};

    for (;;) /* for loop is better in this case, in my view, take or leave it! */
    {
        int num = 0;
        int i;      /* i gets moved to beginning of block as per C89 standard */
        char final_val;

        printf("Line: "); 
        fflush(stdout); /* ensures proper display of "Line: " */

        /*
         * reads from stdin up to but not more that the size that
         * string was set for it.
         * if the first char is a '.' then user is finished
         */
        if (fgets(str, sizeof str, stdin) != NULL && str[0] != '.') {
            /*
             * go through buffer, if the newline or the string terminator is
             * found stop. Newline first since the probability of finding it
             * first is greater.
             */

            for (i = 0; str[i] != '\n' && str[i] != '\0'; i++)
            {
                /* do calculation here */
            }
            /* display results here */
            
        } else {
            break;
        }
    }

    return(0);
}

All that follows the model I previously posted.

Thank you so much for your help Aia. This is what I have put together with your help. The program exits perfectly when the user inputs only a period. However when i input some such as "This line is short." It outputs only a period.

#include <stdio.h>

int
main (void)
{
        char str[50] = {0};
for(;;)
{

        int num = 0;
        int i;
        char final_val;

        printf("Line: ");
        fflush(stdout); // Request Line

        if (fgets(str, 50, stdin) != NULL &&str[0] !='.') {

        for (i = 0; str[i] != '\n' && str[i] != '\0'; i++)
       {
                num+=str[i];
                num = num%64;
                final_val = num;
                num = 0;
       }

       printf("\nChecksum: %c", final_val);
       printf("\n");

        } else {
                break;
        }
        }
        return(0);
}

Is it something wrong with calculation, or have I made a simple error somewhere I cant find.

if (fgets(str, 50, stdin) != NULL &&str[0] !='.') {

I do not know why you insist in using a literal number. sizeof is an operator that will produce the proper capacity every time, regardless if str[] ever change size. That prevents the use of "magic numbers" in programming. A good thing!

printf("\nChecksum: %c", final_val);
printf("\n");

Quite a curiosity! Are you aware that you can include that line-break in the first call to printf(), avoiding the need for a second call? In fact, if you ever are in the need to make a simple line-break, there's a lighter function that will do it for you: putchar('\n'); And for the love anything good! Remove that \n in front of "Checksum:"!


jsphb9000> Is it something wrong with calculation
Remember that "thingy" about "magic numbers", well here's an example of it.

num+=str[i]; /* <-- futile attempt to preserve the num value, it is always initially zero courtesy of the last statement num = 0; */
num = num%64;  /* <-- I don't know what 64 is */
final_val = num; /* <-- int just get demoted back to a char again */
num = 0;    /* <--Why is it set to loose its value? */

I don't know what's the purposed of that block. I know what it is doing as a process, but I cannot tell you if that's the intent, (most likely not, I can guess).
Here's where, some comment describing what the working bits are supposed to do would help; avoiding "magic numbers" that makes sense only to you now, but not tomorrow, also helps.

Edited 6 Years Ago by Aia: n/a

This article has been dead for over six months. Start a new discussion instead.