CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 1 of 3 123 LastLast
Results 1 to 15 of 36
  1. #1
    Join Date
    Feb 2012
    Location
    MD
    Posts
    44

    My very first C program (Windows - console)

    I can't believe I managed to learn this quirky language after all these years.

    Critique away and help me build good coding habits.
    Attached Files Attached Files

  2. #2
    Join Date
    Feb 2003
    Location
    Iasi - Romania
    Posts
    8,234

    Re: My very first C program (Windows - console)

    This is far, far away from a "My very first C program".
    Are you joking or what?
    Ovidiu
    "When in Rome, do as Romans do."
    My latest articles: https://codexpertro.wordpress.com/

  3. #3
    Join Date
    Jul 2005
    Location
    Netherlands
    Posts
    2,042

    Re: My very first C program (Windows - console)

    Quote Originally Posted by jlewand61 View Post
    I can't believe I managed to learn this quirky language after all these years.

    Critique away and help me build good coding habits.
    Macro's are evil, even in C. So if you have to use them, better make misuse as difficult as possible. See http://www.parashift.com/c++-faq-lit....html#faq-39.4

    Furthermore, there are different meaning of the word 'code'. When you write C code, the meaning is not that nobody else should be able to understand it. If you want your code to be understandable (for others as well as yourself) avoid function names like:
    Code:
    void bdla ();
    int fdla ( TCHAR* fdlal );
    void bsopta ();
    ...
    Yes, you documented the functions, but code should be self-documenting first. Source code documentation comes second.

    It's also very hard to find some structure in your code. You have hundreds of functions that appear to be working on the same set of global variables. Most functions are also way too long. It looks like you have a lot of code duplication.
    Cheers, D Drmmr

    Please put [code][/code] tags around your code to preserve indentation and make it more readable.

    As long as man ascribes to himself what is merely a posibility, he will not work for the attainment of it. - P. D. Ouspensky

  4. #4
    Join Date
    Feb 2012
    Location
    MD
    Posts
    44

    Re: My very first C program (Windows - console)

    Quote Originally Posted by ovidiucucu View Post
    This is far, far away from a "My very first C program".
    Are you joking or what?
    ????

  5. #5
    Join Date
    Feb 2012
    Location
    MD
    Posts
    44

    Re: My very first C program (Windows - console)

    Quote Originally Posted by D_Drmmr View Post
    Macro's are evil, even in C. So if you have to use them, better make misuse as difficult as possible. See http://www.parashift.com/c++-faq-lit....html#faq-39.4

    Furthermore, there are different meaning of the word 'code'. When you write C code, the meaning is not that nobody else should be able to understand it. If you want your code to be understandable (for others as well as yourself) avoid function names like:
    Code:
    void bdla ();
    int fdla ( TCHAR* fdlal );
    void bsopta ();
    ...
    Yes, you documented the functions, but code should be self-documenting first. Source code documentation comes second.

    It's also very hard to find some structure in your code. You have hundreds of functions that appear to be working on the same set of global variables. Most functions are also way too long. It looks like you have a lot of code duplication.

    Meaning lengthen the function names so they are understood? Like build_drive_letter_array or something like that?

    I thought the mainline was very structured - function calls (really subroutines). Boom boom boom on down the line.

    Where do you see duplicated code???

  6. #6
    Join Date
    Apr 1999
    Posts
    27,449

    Re: My very first C program (Windows - console)

    Quote Originally Posted by jlewand61 View Post
    ????
    Code:
    #include <stdio.h>
    
    int main()
    {
       printf("Hello World");
    }
    That would be more like a "very first C program".

    Regards,

    Paul McKenzie

  7. #7
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Location
    Florida
    Posts
    12,635

    Re: My very first C program (Windows - console)

    That's definitely not somebody's first program, but I'll comment anyway.

    I find all those comments really distracting and annoying. Give your functions and variables meaningful names so that you don't have to explain them with comments.

    Stop commenting the end of functions, loops, if statements, etc. If you use proper indentation and alignment, that's really easy to see just by looking at the code. //end function completely unnecessary.

  8. #8
    Join Date
    Feb 2012
    Location
    MD
    Posts
    44

    Re: My very first C program (Windows - console)

    Quote Originally Posted by Paul McKenzie View Post
    Code:
    #include <stdio.h>
    
    int main()
    {
       printf("Hello World");
    }
    That would be more like a "very first C program".

    Regards,

    Paul McKenzie
    I think I did do one of those.

  9. #9
    Join Date
    Feb 2012
    Location
    MD
    Posts
    44

    Re: My very first C program (Windows - console)

    Quote Originally Posted by GCDEF View Post
    That's definitely not somebody's first program, but I'll comment anyway.

    I find all those comments really distracting and annoying. Give your functions and variables meaningful names so that you don't have to explain them with comments.

    Stop commenting the end of functions, loops, if statements, etc. If you use proper indentation and alignment, that's really easy to see just by looking at the code. //end function completely unnecessary.
    Understood. But with structured languages like this, you can get lost even with correct indenting. At least I can..... Do mean more verbose variable and function names?

  10. #10
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Location
    Florida
    Posts
    12,635

    Re: My very first C program (Windows - console)

    Quote Originally Posted by jlewand61 View Post
    Understood. But with structured languages like this, you can get lost even with correct indenting. At least I can..... Do mean more verbose variable and function names?
    Yeah. Given this variable I chose randomly, lwbbkpst, nobody would have any clue what it is or what it does. Give it a name that means something.

    You don't line your braces up vertically, which I find annoying. Take this simple function,
    Code:
    void takeownCB ( TCHAR* takeownCBf ) {
    
    
    	// only display non-empty takeown output lines
    	if ( _tcslen ( takeownCBf ) > 1 ) {
    		_tprintf(_T("TAKEOWN  &#37;s"), takeownCBf);
    	}
    
    
    	return;
    
    // end function
    }
    change the indentation
    Code:
    void takeownCB ( TCHAR* takeownCBf ) 
    {
    	if ( _tcslen ( takeownCBf ) > 1 )
            {
    		_tprintf(_T("TAKEOWN  %s"), takeownCBf);
    	}
    	return;
    }
    if you line things up, it's really easy to see where one block, function, etc. starts and ends. Give your variables meaningful names, learn how to use indentation properly and lose 95% of your comments.

  11. #11
    Join Date
    Feb 2012
    Location
    MD
    Posts
    44

    Re: My very first C program (Windows - console)

    Yes, I got in the habit in putting the first squiggly bracket on the right, behind the function/if/do/while/for construct. Simply so it didn't muddy up things and add 1 line of code for every one of those statements. The comments help me so I prefer to keep those in.

    My naming conventions come from decades of assembler programming.

  12. #12
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Location
    Florida
    Posts
    12,635

    Re: My very first C program (Windows - console)

    Well you did say "Critique away and help me build good coding habits.".

  13. #13
    Join Date
    Apr 1999
    Posts
    27,449

    Re: My very first C program (Windows - console)

    Quote Originally Posted by jlewand61 View Post
    My naming conventions come from decades of assembler programming.
    Well, this isn't assembler programming.

    It doesn't cost anything to have meaningful names. If somone were to mantain your code, using cyptic names as you're doing makes that person's job much more difficult.

    Second, when D_Drmmr mentioned duplicate code, more than likely what was being spotted is where code does the same thing, except a different leter is being checked, or a different variable is being used.

    For example:
    Code:
    if ( parmbyt[1] == _T('I') ) {
        if ( pinffl != 0 ) 
            dupparm();
        pinffl = 1;
       // point to parm operand - exit if no more parms
       cargc++;
       if ( cargc >= argc ) exit ( -1 );
    This same code is duplicated, with the only difference being the letter being checked and that pinffl variable. This is where you look carefully as to what that pinffl variable is supposed to do as opposed to the similar variable used in other cases, and whether that entire block of code could be made a function that returns whether there is a duplicate or not.

    Third, in that code above, you had more than one statement per line. If you were to run your code under a debugger, it is difficult to determine whether the if() statement was true or false.

    Fourth, why not use or write a command-line parser in C? There is getopt() and other 'C'-based command parsers, instead of having to reinvent the wheel next time you need to parse the command-line.

    Last, since you're using 'C', there seems to be no check for writing beyond the bounds of those arrays you've declared. You're using Microsoft functions to copy to buffers, but nothing checks if there is room for the copy. You should have written a set of utility functions that wrap these copy functions, so that it ensures that the copying doesn't exceed the bounds of those arrays.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; February 28th, 2012 at 11:18 AM.

  14. #14
    Join Date
    Jul 2005
    Location
    Netherlands
    Posts
    2,042

    Re: My very first C program (Windows - console)

    Quote Originally Posted by jlewand61 View Post
    Meaning lengthen the function names so they are understood? Like build_drive_letter_array or something like that?
    Yes.
    Quote Originally Posted by jlewand61 View Post
    I thought the mainline was very structured - function calls (really subroutines). Boom boom boom on down the line.

    Where do you see duplicated code???
    The structure is lost because your functions are so long and because you use global variables to pass data between functions. Good structure means you can look at a small part of the program in isolation. When you use global variables that becomes nearly impossible, because you don't know what happens to those variables in the rest of the program.

    In your main function, you have some code that looks like this:
    Code:
    while ( cargc < argc ) {
        // ...
        if ( parmbyt[1] == _T('I') ) {
            // some common code
            // some specific code
            cargc++;
            continue;
        }
        if ( parmbyt[1] == _T('T') ) {
            // some common code
            // some specific code
            cargc++;
            continue;
        }
        // etc
    }
    That is, you duplicate a lot of code and to understand the loop logic, I need to look inside each if block. The structure becomes much clearer if you rewrite this as
    Code:
    for (; cargc < argc; ++cargc) {
        // ...
        // common code
        if (parmbyt[1] == _T('I')) {
            // specific code
        }
        else if (parmbyt[1] == _T('T')) {
            // specific code
        }
        // etc
    }
    Now the logic of the program is communicated much better by the code.

    Maybe you can even factor out the specific code into functions and make an array of pointer to those functions. Then call it like
    Code:
    FuncPtr functionTable[256] = { 0 };
    functionTable['I'] = functionForI;
    functionTable['T'] = functionForT;
    // etc
    
    for (; cargc < argc; ++cargc) {
        // ...
        // common code
        functionTable[parmbyt[1]]();
    }
    Cheers, D Drmmr

    Please put [code][/code] tags around your code to preserve indentation and make it more readable.

    As long as man ascribes to himself what is merely a posibility, he will not work for the attainment of it. - P. D. Ouspensky

  15. #15
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,765

    Re: My very first C program (Windows - console)

    Quote Originally Posted by jlewand61
    Yes, I got in the habit in putting the first squiggly bracket on the right, behind the function/if/do/while/for construct.
    I have the same preference as GCDEF when it comes to brace placement, but your style is fine, being a common style, as long as you indent correctly and sufficiently (which you mostly do, except for a slight inconsistency in the use of spaces versus tab characters).

    I think that the benefit of the indentation is somewhat lost when your functions get too long. There are no hard and fast rules as to what is too long, but if I see a function with more than say, 100 lines, I will start itching to see how I can break it down to smaller functions that do one thing and do it well. Likewise, because indentation is a visual guide for scope, global variables lessen its value.
    Last edited by laserlight; February 28th, 2012 at 12:22 PM.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

Page 1 of 3 123 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured