Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

My exercise was:

Write a program that reads input as a stream of characters until encountering EOF. Have the program print each input character and its ASCII decimal value. Note that characters preceding the space character in the ASCII sequence are nonprinting characters. Treat them specially. If the nonprinting character is a newline or tab, print \n or \t, respectively. Print 10 pairs per line, except start a fresh line each time a newline character is encountered.

This is my code (regarding the special characters i defined only \n and \t):

#include <stdio.h>
int special_chars(int ch);

int main(void)

{
    int x;
    printf("please enter a some characters, and ctrl + d to quit\n");

    special_chars(x);

    return 0;
}

int special_chars(int ch)

{
    int pairsNum = 0;

    while ((ch = getchar()) != EOF)// testing charecters while not end of file.
    {

    if (ch == '\n')// testing if a control charecter, and printing its
     {
        printf("\\n ");
        pairsNum++;
     }

    else if (ch == '\t')
     {
        printf("\\t ");
        pairsNum++;
     }

    else
    {
        printf("%c,%d ", ch, ch);
        pairsNum++;
    }

    if (pairsNum == 10)// counting the number of outputs, and printing a newline when is 10 (limit).
        printf("\n");
    }
    return ch;
}

Is it Ok..? tnx

share|improve this question
1  
Surprised this works: printf("%c,%d ", ch, ch); %c is expecting a char while %d an int. While ch is always an int. –  Loki Astari Feb 1 '13 at 19:12
    
Because in C style formatted printing the 'verbs' like c or d don't expect anything, but take at least a word from the stack and format it according to their meaning/specs: c -> character, d -> decimal representation of the int/word. –  Ekkehard.Horner Feb 1 '13 at 21:57
add comment

1 Answer 1

up vote 2 down vote accepted

A few issues, in no particular order:

  • put main last to avoid the need for a prototype.
  • main normally takes argc/argv parameters
  • special_chars should be static - but it is debateable whether a separate function is necessary in a short program like this.

  • main defines variable x and passes it to special_chars. This is unnecessary. Just define ch in special_chars and define the function as taking void (i.e. nothing)

  • why does special_chars return a value?
  • indenting is wrong
  • too many blank lines (8)
  • missing {} brackets after if (pairsNum == 10)
  • mixed naming styles (pairsNum should be pairs_num or special_chars should be specialChars - I prefer the former)
  • special_chars is a bad name for the function.
  • pairsNum is an odd name; n_pairs would be my choice.
  • you don't print a final \n at the end of the program
  • you don't start a new line after reading a \n, as required - it appears that you do when you type at the keyboard because entering a \n causes a new line. But if you redirect input into your program it does not. Also you don't start a line after each ten, only after the first ten. e.g. if your program executable is called program

    $ cat x
    abcdefgh
    ijklmnop
    qrstuvwxyz
    
    $./program < x
    please enter a some characters, and ctrl + d to quit
    a,97 b,98 c,99 d,100 e,101 f,102 g,103 h,104 \n i,105 
    j,106 k,107 l,108 m,109 n,110 o,111 p,112 \n q,113 r,114 s,115 t,116 u,117 v,118 w,119 x,120 y,121 z,122 \n $ 
    
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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