5
\$\begingroup\$

While I was coding it, I got a better idea, but I continued because I can not compare both of those ideas, because they are different.

The idea of that function is to make your fake ui-building easier AND more advanced.

void
write_ui
(char nodes[], int FOREGROUND, int BACKGROUND)
{
    int i;
    char buff[strlen(nodes)];

    strcpy(buff, nodes);
    HANDLE Handle = GetConsoleWindow();
    SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND | BACKGROUND);

    for(i=0; buff[i] != '\0'; i++)
    {
        if(buff[i] == '-') { printf("%c", 196); }                                                      // Horizontal line
        else if(buff[i] != '|' && buff[i] != '_' &&
                buff[i] != '^' && buff[i] != '~') { printf("%c", buff[i]); }                         // Any other character

        if(buff[i] == '|' && buff[i-1] != '-' && buff[i+1] != '-' &&
           buff[i-1] != '_' && buff[i+1] != '^' && buff[i-1] != '^') { printf("%c", 179); }          // vertical line  && !isdigit(buff[i])

        if(buff[i] == '|' && buff[i+1] == '_') { printf("\n%c", 192); }                               // bottom right corner
        if(buff[i] == '_' && buff[i+1] == '|') { printf("%c", 217); }                                 // bottom left corner
        if(buff[i] == '|' && buff[i+1] == '^') { printf("%c", 218); }                                 // top left corner
        if(buff[i] == '^' && buff[i+1] == '|') { printf("%c", 191); }                                 // top right corner

        if(buff[i] == '-' && buff[i+1] == '|') { printf("%c", 180); }                                 // ┤
        if(buff[i] == '|' && buff[i+1] == '-') { printf("%c", 195); }                                 // ├

        if(buff[i] == '~')
        {
            int spaces = 0, len = 0, l = 0, multiplier = 1;

            do { len++; multiplier *= 10; } while(isdigit(buff[i+(len+1)])); multiplier /= 10;
            l = len;
            do { spaces += (chrtodigit(buff[(i+len+1)-l]) * multiplier); l--; multiplier /= 10; } while( l != -1 );

            splice_away(buff, i, i+len);                                                            // To cut the number passed as number of spaces from screen

            l = 0;

            do { l++; putchar(' '); } while(l != spaces);
        }
    }
}

int chrtodigit (char chr)
{
    if(chr >= '0' && chr <= '9') { return chr-48; }
    else return 0;
}
int intlen (int integer)
{
    int number, len;
    for(number = integer; number != 0; len++, number/=10);
    return len;
}
void splice_away(char str[], int from, int to)
{
    memmove(str + from, str + to, 1 + strlen(str + to));
}

Example:

int main()
{
    int ui1=80;

    HANDLE Handle = GetConsoleWindow();

    SetWindowPos(Handle, HWND_TOP, 50, 50, 1000, 500, SWP_DRAWFRAME);
    SetConsoleTitle("E X A M P L E"); system("COLOR 1B");
    SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), 9 | BACKGROUND_BLUE);
    while(ui1-->0) putchar(205);
    SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), BACKGROUND_RED);
    write_ui("|^----------------------^|\n|Test box - fake UI~4|\n|Written manually~6|\n|~22|_--------| Test |------_|", 1, 1);
    Sleep(10000);

    return 1;
}

Result:

Cropped screenshot of fake UI window

The function is capable of a lot. I would like to see some UIs created with this function.

Here is how you can use the function:

String escape sequences:

  • | For vertical line
  • - For horizontal line
  • -| For left pause (break for titles etc)
  • |- For right pause
  • |_ Bottom left corner
  • _| Bottom right corner
  • |^ Top left corner
  • ^| Top left corner
  • ~x Any number to represent count of spaces
\$\endgroup\$
5
\$\begingroup\$
(char nodes[], int FOREGROUND, int BACKGROUND)

Your function does not modify nodes so it should be declared const (const char nodes[]).

Normally, all-uppercase identifiers are used for macros, not function parameters. Consider calling these foreground and background.

do { spaces += (chrtodigit(buff[(i+len+1)-l]) * multiplier); l--; multiplier /= 10; } while( l != -1 );

Code like the above is trying way too hard to write loops in one line. This harms readability because it's not clear what the loop body is and what the terminating condition is. Suggest writing loops like this on multiple lines. There are other instances of this such as the loop in intlen.

In your handling of ~, you can combine the two loops that measure and then convert the integer, into one. Or, call a function such as strtol() that does the job for you.

You don't need the splice_away() function at all. You can replace the call to it with i += len to skip over the integer in the input. This also means you don't need buff, and can use nodes directly instead of copying it to buff.

intlen() does not appear to be used anywhere in your code. Also, it doesn't properly handle negative numbers.

while(ui1-->0) putchar(205);

The above appears to be a gratuitous use of the --> operator and should probably be written in a more conventional style. For example, for (ui1 = 0; ui1 < 80; ui1++).

\$\endgroup\$
  • \$\begingroup\$ Oh yes. I know.. intlen() was because firstly i intended to count digit of integers \$\endgroup\$ – Edenia Oct 19 '14 at 20:38
  • \$\begingroup\$ I disagree about while(ui1-->0) putchar(205); should be overwritten with a for loop. the term while(ui1-->0) looks much more brief and self-explaining. It is a gratuitous use of the "goes to" operator though. \$\endgroup\$ – Edenia Oct 19 '14 at 20:43
  • \$\begingroup\$ Well, except for the fact that the number of characters it's writing (declared as int ui1=80) is several lines away from the loop and changes the value too, so you can't immediately use the same value to say, write a second horizontal line across the screen below the box. \$\endgroup\$ – Greg Hewgill Oct 19 '14 at 20:45
  • \$\begingroup\$ I know.. and i won't. I am not even sure why do i have that line.. maybe planed to do a menu or.. \$\endgroup\$ – Edenia Oct 19 '14 at 20:57
  • \$\begingroup\$ hm.. is there any function to obtain the color given by the system call "COLOR" ? \$\endgroup\$ – Edenia Oct 19 '14 at 21:23
3
\$\begingroup\$

A few minors

  1. ALLUPPERCASE in C is usually used for macros and not function parameter names.

  2. Rather than making a copy of the passed in character sequence you could use a pointer to iterate over it. This would man you can get rid of the splice_away function as you could simply increment the pointer by the required index. Something like this:

    char *current_char = nodes;
    
    while (*current_char != '\0')
    {
        // do your logic in here and instead of
        // splice_away you can simply do current_char += len;
    }
    
\$\endgroup\$
2
\$\begingroup\$

A few other things that weren't mentioned:

Your code is very Windows specific. I would like to test on my mac though ;). If you are up for it, maybe you can make it portable by replacing the Win sys calls with ANSI color escape codes. See this and this. Or maybe switch to a more powerful library such as ncurses.

Other details:

  • Use of VLA in char buff[strlen(nodes)];. This is considered poor practice by some, as it can be a source of stack overflows. Consider using a dynamic buffer acquired with malloc().

  • You need to be more consistent with your naming and code layout. You have some names in a tightlypacked notation, e.g.: chrtodigit, and others using snake_case, e.g: splice_away. I suggest sticking to the latter, as it seems more readable. Also, the declaration of write_ui() was split in 3 lines, one for the return type, one for the name and the arg list in its own line. Other functions don't follow this convention. Style consistency is quite important. It helps the "human compiler" process the code.

  • Why exactly did you return 1 from main()? It should return zero or nothing on success. Any other value is to indicate an error with the program.

  • Long lines were already mentioned. That is a common source of confusion and possibly bugs. You should aim at an 80 columns soft limit and 120 cols hard limit. That is my personal metric.

\$\endgroup\$
  • \$\begingroup\$ I may switch to ncurses. But if i do that.. that function will be still valuable? I return 1 because if the main function successfully get into that line.. then the function is ok and should return 1. But its hardware-specific. And im using C::B along gcc with c11 \$\endgroup\$ – Edenia Oct 19 '14 at 22:49
2
\$\begingroup\$

Instead of using printf("%c",n), consider using putc(n).

Also, consider using some sort of higher-level lookup table to do your mappings, where appropriate; there's only so much you can do with pure C but you could still build, for example, an array of lambdas or function pointers that map characters to handler functions.

Many of your complex comparisons could be made a bit more concise and possibly more readable using strcmp.

In chrtodigit, it's much better to just write it as:

int chrtodigit(char c) {
    if (c >= '0' && c <= '9') {
        return c - '0';
    }
    return 0; // or better yet, return an error code that gets checked...
}

Remember that in C, a character literal is an int. No need to look up the actual integer value in an ASCII table when you already have it right there!

\$\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.