Krdytkyu

Here is my code where I have 3 functions that do simple arithmetic:

#include <stdio.h>



/* The following Code Contains 3 functions

 * that compute simple arithmetic such as

 * computing the sum of 3 integers and divison

 * of three floating integers.

 */

 void getSum(int x, int y, int z);

 double doDiv(float one, float two, float three);

 void getDiv(float a, float b, float c);



 int main()

 {  

    getSum(100,242,62); 

    getDiv(100.0,25.0,2.0);

    return 0;

 }



/*The function getSum accepts three int

 *values and computes the sum of all three and prints

 *the result via standard output.

 */



 void getSum(int x, int y, int z)

 {

    int sum = x + y + z;

    printf("%in",sum);



 }



/*The function doDiv accepts three floating

 *values and computes the divison of first two.

 *Finally it divides the result by the third 

 * floating value and returns the final answer.

 */



 double doDiv(float one,float two, float three)

 {

    float answer; 

    answer = one/two;

    answer = answer/three;

    return answer;

 }



/* The function getDiv accepts three floating values

 * and then calls the doDiv function. The three accepted 

 * values are passed as parameters in the function call.

 */



 void getDiv(float a, float b, float c)

 {

    float finalAnswer;

    finalAnswer = doDiv(a,b,c);

    printf("%fn", finalAnswer);

 } 

I have tried my best to comment each function but I am not sure if I am doing it correctly. Can someone please advise me on making my comments better, naming variables better and anything else that can help me utilize coding standards correctly?

I would like to pick up the habit now while I am still writing small lines of code so I can take it forward with me when I write bigger codes.

Any suggestions on how I can comment better, name variables better, write code more elegant are welcome. Also, do I comment the method declarations on top?

There's a bit to say about this code, so I'll try to take it from the top and work my way down:

• Declare main() after your functions so you don't have to declare those function prototypes at the beginning of your source code.




• You shouldn't be printing anything from the functions other than main() here. You should be returning the values to main() and handle them there.




• You are restricting yourself by only accepting int parameters into your getsum() function. I would use intmax_t from <stdint.h> instead.




• You don't protect yourself from integer overflow within any of your functions. Take a look at this question and answer for an idea of how to implement that protection using the latest standards.




• Why does your doDiv() function accept float values, and then return a double value? Inconsistent and not what a programmer would expect. Either return a float, or accept doubles as parameters.



• 
  

  Always declare what parameters your function takes in, even if nothing.

  
  
  

  int main(void)
  
  

  
  
  

  You might wonder why we have to do this. Imagine we have the function foo() declared as such:

  
  
  

  int foo()
  
  

  
  
  

  In C, this is known as an identifier list and means that it "can take any number of parameters of unknown types". We can actually pass values to the function even though we don't mean to or intend to. If the caller calls the function giving it some argument, the behavior is undefined. The stack could become corrupted for example, because the called function expects a different layout when it gains control.

  
  
  

  Using identifier lists in function parameters is depreciated. It is much better to do something like:

  
  
  

  int foo(void)
  
  

  
  
  

  In C, this is known as a parameter type list and defines that the function takes zero arguments (and also communicates that when reading it) - like with all cases where the function is declared using a parameter type list, which is called a prototype. If the caller calls the function and gives it some argument, that is an error and the compiler spits out an appropriate error.

  
  
  

  The second way of declaring a function has plenty of benefits. One of course is that amount and types of parameters are checked. Another difference is that because the compiler knows the parameter types, it can apply implicit conversions of the arguments to the type of the parameters. If no parameter type list is present, that can't be done, and arguments are converted to promoted types (that is called the default argument promotion). char will become int, for example, while float will become double.

  
  



• Use %g when printing float values. It also allows you to print double if you decide to "upgrade" the type later.



• 
  

  You don't have to return 0 at the end of main(), just like you wouldn't bother putting return; at the end of a void-returning function. The C standard knows how frequently this is used, and lets you not bother.

  
  
  

  
  

  C99 & C11 §5.1.2.2(3)

  
  
  

  ...reaching the } that terminates the main() function returns a
  value of 0.

  
  

  
  



• You asked me to comment on your formatting and documentation. I find leading by example is a good way to learn something, so look at how I do it in this question of mine. Also, I use doxygen for my documentation generation. I think it is a good habit that you should get into as well.

Comments should tell the reader something useful and non-obvious. Your comments are mainly noise, partly because the functions are so trivial that they need no comment. As others have said, making the function name descriptive can often remove the need for comments, but I dislike very long function names. A getSum function could be sum_xyz and be quite clear. Your function names are bad (eg getSum doesn't 'get' the sum but instead prints it).

Dissecting a comment:

/*The function getSum accepts three int

 *values and computes the sum of all three and prints

 *the result via standard output.

 */

Starting with "The function getSum"; this tells me nothing. I know you are describing getSum because the comment precedes it. The "accepts three int values" is clear from the prototype and tells me nothing new. The most that need be said (if it were not already obvious from the code) would be:

/* print sum of inputs on stdout */

Similarly, doDiv would be:

/* return (x/y)/z */

and getDiv:

/* print (x/y)/z on stdout */

Note that people disagree on commenting, some liking more than others. Often you will have to fit in with what your employer dictates. Some people like Doxygen, which has been suggested above. Personally, I don't - a Doxygen version of your function header might be:

/**

 * @fn void getSum(int x, int y, int z)

 * @brief print sum of inputs on stdout

 * @param x 1st input

 * @param y 2nd input

 * @param z 3rd input

 */

Using this you get your function included in a printable document of some sort but the cost is high (look at the redundancy! Also how ugly, compared to print sum of inputs on stdout) and you can be fairly sure that nobody maintaining your function (e.g. changing it at a later date to divide by 3) will touch the comment, leaving it progressively more out of date. The other thing about Doxygen comments is that people spend inordinate amounts of time getting their printed documentation to look "just right".

The comment to the last function getDiv is bad. I can't figure out what it does by reading the comment, only by reading the code itself. For example:

/* getDiv prints the result of dividing a by b and then by c. */

Here's a crazy idea. What if you code eliminated most of these comments and instead became self-documenting?

Is this dark magic, plain nonsense or actually viable for production code? Let's take a stab at it.

Let's simply change your function names and parameters and see if it makes a difference.

(UPDATED first function name due to error pointed out in comments.)

 void printSumOfThreeNumbers(int first, int second, int third);

 double divideFirstNumberBySecondAndThenThird(float first, float second, float third);

 void divideFirstNumberBySecondAndThenThirdAndPrintResult(float first, float second, float third);

Now I'm willing to bet you can delete EVERY COMMENT in your original code snippet and most sensible people will not whine about an apparent lack of comments; your code's intention is expressed rather well in the names alone.

A lot of people reading code would prefer the above instead of some fluffy flavor text that will very soon rot; meaning that the next guy who comes along may change your (private) functions, add/subtract parameters and not even bother to change your elaborate comments rendering them ineffective at best and inaccurate/misleading at worst.

By letting your code read better with meaningful function and variable names, you are doing your readers a favor.

Software is often a write-once, read-many-times activity.