Funky output

**zeldac** · 09-16-2014

I am trying to alphabetize a list from user input. However when it prints the ordered list I get like letters, it's not completely junk, but it still isn't right. It's like a combination of the words I just input and input from the last time I ran the program, or the words kinda jumbled up. I'm new to programming and was just wondering if someone saw something I wasn't. Thank you

Code:

#include <stdio.h>
#include <string.h>

 int main()
{
     int i, j;
    char  ch, string[10][20], temp[20];
    int num;
    j=0;
    i=0;


    while(1)
    {
        ch = getchar();
        if(ch <= 0x39 && ch >= 0x30)
        {
            putchar(ch);
            num = ch - 0x30;
        }
        if(ch == 0x0D)
             printf("\n\r");
            break;
    }


    while(i <= num)
    {
        ch = getchar();


        if (ch <= 0x7A && ch >= 0x61)
        {
            putchar(ch);
            string[i][j] = ch;
            j++;
        }
        if(ch == 0x0D)
        {
            string[i][j] = '\0';
             printf("\n\r");
            i++;
        }
    }
    for (i = 0; i < num - 1 ; i++)
    {
       for (j = i + 1; j < num; j++)
       {
           if (strcmp(string[i], string[j]) > 0)
           {
               strcpy(temp, string[i]);
               strcpy(string[i], string[j]);
               strcpy(string[j], temp);
           }
       }
    }
     printf("\nSorted Names\r");
    for (i = 0; i <= num; i++)
    {
        printf("%s\n\r", string[i]);
    }
    return 0;
}

**Click_here** · 09-16-2014

Code:

// Should this
        if(ch == 0x0D)
             printf("\n\r");
            break;

// Be this?

    if(ch == 0x0D)
    {
      printf("\n\r");
      break;
    }

It's also a good idea to initialise your arrays

Code:

char  ch, string[10][20]={{0}}, temp[20]={0};

**anduril462** · 09-16-2014

Your indentation is a mess, in the future, please post code that is properly indented/formatted. It will help you keep things straight, and it will also make our job of helping you easier. Using the auto-indent feature of your favorite code editor is a good way to keep things neat and organized.

Give us sample input that demonstrates the problem, along with the incorrect output you see and the correct output you want to see. That way we can be sure to replicate your problem, and help you fix it. This also ties in with my first bullet point below.

For example, you have:

Code:

        if(ch == 0x0D)
             printf("\n\r");
            break;

But after auto-indenting, I get

Code:

        if(ch == 0x0D)
            printf("\n\r");
        break;

Note how the break is not actually part of your if statement. Unlike Python, C does not care about indentation. You need to use curly brackets if you want to group multiple statements under an if condition.

Some other problems I see (this is from a quick visual, I may have missed some):

Print prompts that tell the user what kind of data to input. I didn't try to find any runtime bugs because I had no idea how to run it or what input to enter.
Don't use magic numbers. Why 10 and 20 for your arrays? #define constants with sensible names like MAX_WORDS and MAX_WORD_LEN.
In the same vein, WTF are 0x39 and 0x30? It would be better to use '9' and '0', but even better still if you used the isdigit() function -- just remember to #include <ctype.h>. Similar for 0x7A and all the others. There are lots of useful functions: isalpha(), islower(), isupper() and many more. Look into all the functions provided by ctype.h. Also, what is 0x0D? EOF? Try checking for EOF explicitly.
Try to avoid a while(1) with a break statement. Use the correct loop condition to terminate, it's much clearer. Also, you may want to consider a do-while loop here since it seems you want to ensure your loop runs at least once.
getchar() returns an int. Thus, ch should be declared an int. This is so getchar can return all possible char values, plus special values like EOF.
Instead of mucking around with all these complicated loops with getchar, why not use something like scanf to read a word (%s) or fgets to read a whole line? These will also allow you to limit the length of data you read in, to avoid overflowing your arrays (which results in undefined behavior, meaning anything can happen, such as a seg fault).

That's plenty to work on for now.

EDIT: I mention using the functions in ctype.h for a few reasons. They make it clearer what you are trying to check. Also, you should be aware, there are other character sets besides ASCII (and compatible) -- they're very rare nowadays, but they do exist. For example, in EBCDIC, the digits are in a different location, as are the letters. Furthermore, the letters aren't contiguous! So you can't check it with a simple < and >

**zeldac** · 09-16-2014

Originally Posted by Click_here

It's also a good idea to initialise your arrays

Code:

char  ch, string[10][20]={{0}}, temp[20]={0};

oops, first one is just human error. But thank you for this advice, although fixed the garbage printing, it is only printing one word from the list now :/

**zeldac** · 09-16-2014

Originally Posted by anduril462

Give us sample input that demonstrates the problem, along with the incorrect output you see and the correct output you want to see. That way we can be sure to replicate your problem, and help you fix it. This also ties in with my first bullet point below.

sample input:
4
one
two
three
four

sample output:
Sorted Names

one

**Click_here** · 09-16-2014

Can you post your current code?

**zeldac** · 09-17-2014

Originally Posted by Click_here

Can you post your current code?

Code:

 int main(){
    init_platform();


    int i, j;
    char  ch, string[10][20]={{0}}, temp[20]={0};
    int num;
    j=0;
    i=0;


    while(ch != 0x0D)             //get number from user
    {                                     //this number will be number of words sorted
        ch = getchar();
        if(ch <= 0x39 && ch >= 0x30)
        {
            putchar(ch);
            num = ch - 0x30;
        }
        if(ch == 0x0D)
        {    printf("\n\r");


        }
    }


    while(i < num)                                   //save characters into string [][]
    {


        ch = getchar();


        if (ch <= 0x7A && ch >= 0x61)
        {
            putchar(ch);
            string[i][j] = ch;
            j++;
        }
        if(ch == 0x0D)
        {
            string[i][j] = '\0';
            printf("\n\r");
            i++;
        }
    }
    
    for (i = 0; i < num - 1 ; i++)                       //compare and sort to alphabetize
    {
       for (j = i + 1; j < num; j++)
       {
           if (strcmp(string[i], string[j]) > 0)  
           {
               strcpy(temp, string[i]);
               strcpy(string[i], string[j]);
               strcpy(string[j], temp);
           }
       }
    }
    printf("\nSorted Names\r");    //print sorted words
    for (i = 0; i < num; i++)
    {
       printf("%s\n\r", string[i]);
    }
    return 0;
}

I apologize for no print prompts, the use of hexidecimal and getchar, I'm just following my assignments instructions

also it seems to be only printing out one word now.
input:
3
one
two
three

output:
Sorted Names

one

**phantomotap** · 09-17-2014

I'm just following my assignments instructions

O_o

Send him/her by, I'd love to ask why poor practices are being taught.

Soma

**zeldac** · 09-17-2014

Originally Posted by phantomotap

O_o

Send him/her by, I'd love to ask why poor practices are being taught.

I'm sure he just wants us to think in a different way... since you can find code easily online to alphabetize a list, that's just what I think this is.

**anduril462** · 09-17-2014

Originally Posted by zeldac

I'm sure he just wants us to think in a different way... since you can find code easily online to alphabetize a list, that's just what I think this is.

Yes, it's easy to find the code on line, but that is not what Soma was referring to. Teaching you to use 0x39 instead of '9', or the isdigit() function, is just teaching bad practices. The only way it might teach you to think differently, is that it teaches you to write code that is difficult to read and maintain. In the real world (most decent schools and virtually all professional settings), you would not be allowed to submit such code.

As for why your code doesn't work, try printing the value of i and j every time the user enters a new word. See if you see a problem.

**zeldac** · 09-17-2014

Originally Posted by anduril462

Yes, it's easy to find the code on line, but that is not what Soma was referring to. Teaching you to use 0x39 instead of '9', or the isdigit() function, is just teaching bad practices. The only way it might teach you to think differently, is that it teaches you to write code that is difficult to read and maintain. In the real world (most decent schools and virtually all professional settings), you would not be allowed to submit such code.

As for why your code doesn't work, try printing the value of i and j every time the user enters a new word. See if you see a problem.

I think the use of hex has to do with the microprocessor we use, but thank you, I will keep in mind to use the other methods in the future.
As for the value of i and j, I'm starting to realize that string[i][j] might only be saving the first word of the list? Any ideas of how to fix this?

**anduril462** · 09-17-2014

Did you try printing them out? What did you notice about the value of j?

EDIT: Try printing i and j on every iteration of the loop.

**Matticus** · 09-17-2014

I think the use of hex has to do with the microprocessor we use, but thank you, I will keep in mind to use the other methods in the future.

Any such compiler should support character literals.

It's all about context.

If you read a byte from the UART, and you want to know if it's the letter B, you check for 'B' since it's more clear in that context than 0x42 (and you don't have to check a table for the value to compare against).

On the other hand, if you're sending a byte to a port controlling an LED array (7-0), and you want to light LEDs 6 and 1, you use 0x42, because it's more clear for that context.

**zeldac** · 09-17-2014

Originally Posted by anduril462

Did you try printing them out? What did you notice about the value of j?

EDIT: Try printing i and j on every iteration of the loop.

when I print out j, I'm getting a different letter than I input?

**anduril462** · 09-17-2014

I'm not talking about letters. Don't print string[i][j]. Print the actual values of i and j -- the numbers corresponding to which word (i) and which letter/position of that word (j) you're storing the letter in. Then maybe it will become clear. In other words, add this line to your loop:

Code:

printf("i=%d, j=%d\n", i, j);

Alternatively, use a debugger to watch the values of i and j as you step through the program.

Thread: Funky output

Thread Tools

Search Thread

Display

Funky output

Similar Threads

Funky enum

Funky Error

output produces funky characters

Funky new problems?

funky new avatar