1
\$\begingroup\$

My task was:

Write a function that takes three arguments: a character and two integers. The character is to be printed. The first integer specifies the number of times that the character is to be printed on a line, and the second integer specifies the number of lines that are to be printed. Write a program that makes use of this function.

And my code is: (i know i did not add different scenarios like if the first char is number etc, but please let me know if it looks ok)

#include <stdio.h>
void printing_char (char ch, int numberOfChars, int numberOfLines);

int main (void)
{
    char userChar;
    int lines,times;
    printf ("please enter a character, number of times in a line, and number of lines:\n");
    while ((scanf ("%c%d%d", &userChar, &times, &lines)) == 3)
    {
        printing_char (userChar, times, lines);
    }
    return 0;
}

void printing_char (char ch, int numberOfChars, int numberOfLines)
{
    int x;
    int y = 0;

    while (++y <= numberOfLines)
    {
            for (x = 0; x<numberOfChars; x++)
            {
                printf ("%c", ch);
            }
            printf ("\n");
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ printf("%c", ch); have you considered putchar(ch);? \$\endgroup\$ – asveikau Feb 5 '13 at 5:09
3
\$\begingroup\$

Only a couple of things:

  • int lines,times; - one declaration per line, please!

  • scanf("%c%d%d", &userChar, &times, &lines) - where does one number end and the next start?. You need to include some terminating characters, such as ,

    printf("please enter a character, number of times in a line, and number of lines, separated by a comma:\n");
    while ((scanf("%c,%d,%d", &userChar, &times, &lines)) == 3)
    
  • An expression such as while (++y <= numberOfLines) is confusing, what value does y have at each step? It'd be better to rewrite this as a for loop

    for ( y = 0; y < numberOfLines; y++ )
    
\$\endgroup\$
1
  • \$\begingroup\$ One number ends and the next starts at whitespace - remember that numeric conversions skip leading whitespace. \$\endgroup\$ – Toby Speight Oct 24 '18 at 14:21
2
\$\begingroup\$

In addition to comments by @GlennRogers, here's a bit of pedantry:

  • consider putting main last to avoid the need for a prototype for printing_char

  • printing_char should be static. Seems a bad name too.

  • for-loops can define the loop variable in the initial loop:

    for (int x = 0; x<numberOfChars; x++)
    
  • nested loops are generally best avoided. Your inner for-loop would be better as a function:

    static void print_n_times(int ch, int n)
    {
        for (int i=0; i<n; ++i) {
             putchar(ch);
        }
    }
    
  • adding a blank line after the function definition is very odd. Best not done.

  • numberOfChars and numberOfLines are verbose for my taste. I prefer n_chars, n_lines (or camel-case nChars, nLines). Also, as the variables in main have exactly the same meaning, it would be reasonable to call lines and times the same there, ie. n_chars and n_lines

  • you have unnecessary brackets around the scanf call

\$\endgroup\$
0
\$\begingroup\$

Expensive operations here are the nested loops and the multiple calls to the printf function.

These hotspots can be corrected by using the memset and memcpy functions (but be careful, you can quickly misuse them).

Only one printf and one loop

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

void printing_char (char ch, int length, int lines)
{
    if (length < 1 || lines < 1) return;

    int size = length+1;
    char str[size*lines+1];

    memset(str, ch, length);
    str[length] = '\n';
    str [size*lines] = '\0';

    for (int off = size*(lines-1); off > 0; off -= size) {
        memcpy(str + off, str, size); 
    } 
    printf ("%s %d", str, lines);
}

int main (void)
{
    char ch;
    int lines;
    int times;

    printf("please enter a character, number of times in a line, and number of lines, separated by a comma:\n");
    while ((scanf("%c,%d,%d", &ch, &times, &lines)) == 3) {
        printing_char (ch, times, lines);
    }
    //printing_char('*', 4, 3);
    //printing_char('^', 0, 5);
    //printing_char('.', 3, -1);

    return 0;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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