1
\$\begingroup\$

This is a program you can compile on Windows. The program is an invitation of a chat system for programmers. Once you press enter, you will open the link with your default Internet browser. You can learn more from the program itself.

  • 102 lines of code
  • 4 377 bytes size of source file
  • 192 984 bytes size of binary executable file
  • Do note that the program requires administrative privileges

#include <stdio.h>
#include <windows.h>

#define color       "78"
#define title       "E-Chat Invitation By Al3"
#define KEY_ENTER   0x0D
#define KEY_I       0x49

#define echo        "ECHO"

typedef enum OPTIONS {LAUNCH = 10, INFO = 20} option;

int
main
(void)
{
    option main_option;
    HANDLE Handle = GetConsoleWindow();
    FILE* web;

    system("COLOR " color);
    system("TITLE " title);

    menu :
    SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), 9 | BACKGROUND_RED); // more simplified coloring
    system(echo " Press Enter to launch the e-chat or press i for info\n");
    SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), 143); // combination of 4 bit fg and 4 bit bg
    main_option = 0;

    while(1)
    {
        if(GetAsyncKeyState(KEY_ENTER)) {   main_option = LAUNCH;   }
        if(GetAsyncKeyState(KEY_I))     {   main_option = INFO;     }

        if(main_option &&
           !GetAsyncKeyState(KEY_I) &&
           !GetAsyncKeyState(KEY_ENTER))
        {
            break; // Exits the loop only if an option is selected and the key is not bussy
        }
    }

    switch(main_option)
    {
        case LAUNCH:    system("CLS"); printf("Launching..\n");
                        web = fopen("temporal_weblink_echat.url", "w");
                        fputs("[InternetShortcut]\n"
                              "URL=http://e-chat.co/room/34017/\n"
                              "IDList=\n"
                              "HotKey=\n"
                              "IconFile=\n"
                              "IconIndex=0", web);
                        fclose(web);
                        {
                            int
                            execution = ShellExecute(NULL, "open", "temporal_weblink_echat.url", NULL, NULL, SW_SHOWNORMAL);

                            if(execution <= 32)
                            {
                                char str1[256];
                                char str2[256];

                                sprintf(str1, "Cannot open temporal file. File may have been moved or deleted. Admin rights are required.");
                                sprintf(str2, "Error with error code (%02i)", execution);

                                MessageBox (NULL, str1, str2, MB_OK | 0x00000010L);

                                goto menu;
                            }
                        }
                        if(remove("temporal_weblink_echat.url"))
                        {
                            printf("Could not delete the temporal file, please try again.\n\n");
                            goto menu;
                        }
        break;
        case INFO:      system("CLS");
                        printf("The e-chat is an Online 24/7-running public chat room,\n"
                               "dedicated for programmers, programming in the language C.\n"
                               "Feel free to come at any time if you have any C-related questions.\n"
                               "Note that the e-chat has no sound notification yet,\n"
                               "which means sometimes people can not response at the moment you type something.\n"
                               "No registration is needed,\n"
                               "you can enter as a guest only by passing\n"
                               "the name you want to identify yourself. If you share the link to more people,\n"
                               "the e-chat might become a certain environment for immediate collaboration\n"
                               "when it comes to programming on that particular language.\n\n"
                               "Rules are simple:\n"
                               "1. Do not discuss anything that has nothing to do with C programming.\n"
                               "   - Chat ban follows\n"
                               "2. Do not harass people.\n"
                               "   - Anyone can use an option \"Ignore\" for that purpose.\n"
                               "3. Try using proper and understandable form of communication.\n"
                               "   -Chat kicks as a warning are possible.\n\n"); goto menu;
        break;
    }

    return 0;
}
\$\endgroup\$
  • 3
    \$\begingroup\$ I have to say, asking users to run an untrusted binary executable that requires administrative privileges just to open a URL in a web browser sounds like a horrible idea. \$\endgroup\$ – 200_success Nov 2 '14 at 4:38
  • \$\begingroup\$ I can't believe people don't get the real idea. \$\endgroup\$ – Edenia Nov 2 '14 at 10:45
  • \$\begingroup\$ Here's something I noticed by taking a quick look: poor error checking/handling, system calls, gotos like that easily makes the code messy \$\endgroup\$ – Okkaaj Nov 6 '14 at 19:20
3
\$\begingroup\$

Your code has serious readability issues. And that is important. That's the reason we invented C in the first place.


This:

system(echo " Press Enter to launch the e-chat or press i for info\n");

Why did you use a define for echo? That looks very bizarre and unnecessary. But why even do that? Forward printing to an external process? Why not just printf?


"Magic" constants:

SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), 9 | BACKGROUND_RED);
SetConsoleTextAttribute (GetStdHandle(STD_OUTPUT_HANDLE), 143);

What's the 9 and 143 supposed to mean? Isn't there a #define for those on Windows.h?

Same here:

MessageBox (NULL, str1, str2, MB_OK | 0x00000010L);

Inconsistency with macro naming:

#define color       "78"
#define title       "E-Chat Invitation By Al3"
#define KEY_ENTER   0x0D
#define KEY_I       0x49

color and title should follow the ALL_CAPS macro/constant convention: COLOR and TITLE.


They way you've indented the switch(main_option) is pretty bad. So many levels of indentation for nothing. Rearrange that in a more plausible way:

switch (main_option)
{
case LAUNCH:
    ... stuff ...
    break;

case INFO:
    ... stuff ...
    break;

default:
    break;
} // switch (main_option)

You never check the result of web = fopen("temporal_weblink_echat.url", "w");. That's undefined behaviour on the call to fputs if fopen fails and you use the bad FILE.


Not mixing { } positioning styles would make your code easier on the eyes of the reader. Try to be consistent with a single style. Avoid things like this:

if(GetAsyncKeyState(KEY_ENTER)) {   main_option = LAUNCH;   }

When the rest of the code is written like:

if (x)
{
    do_y();
}

Heavy use of goto can lead to what is called Spaghetti Code. It would be wise to put some thought into refactoring your program to use more subroutines and avoid goto.

\$\endgroup\$
  • \$\begingroup\$ Used goto once like that won't result in SC. I forgot to check these, thanks for reminding me. while(1) and for( ; ; ) are absolute the same except that while(1) is more "readable" as you obviously pretty much rely on. I use switch statements on a normal way. No extra styles. Yes i need to keep macro's form. Will make them all with capital letters. Actually using { and } like that looks much more readable to me. Ahh magic numbers.. They are not even important why do i have to fill my code with more code when it is just.. pointless. \$\endgroup\$ – Edenia Nov 2 '14 at 3:48
  • \$\begingroup\$ Also.. Defining a macro for that kind of magic number is not a good idea. That magic number if known.. is good to be modifiable. But its not, unless you have some extra knowledge of how MS deal with console colors. \$\endgroup\$ – Edenia Nov 2 '14 at 3:50
  • \$\begingroup\$ @Edenia - I'd say that while(1) reads just as strangely as for(;;). I removed that part. It was irrelevant, really. If you use booleans (stdbool.h) then while(true) would be the most idiomatic form, I guess. But yes, they are all the same on modern compilers. \$\endgroup\$ – glampert Nov 2 '14 at 4:19

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.