4
\$\begingroup\$

Can my code be reviewed by common C++ formatting standards, and can it be optimized? My code determines if the number (x) you input is greater then the other number (y) you input, the code then prints out the results.

#include <iostream>
#include <stdio.h>

using namespace std;

int oper();

int main() {
    oper(); //function for the operation. 
}
int oper() {
    int x = 0;
    int y = 0;
    int c;

    std::cin >> x; 
    std::cin >> y;

    if (x > y)
        cout << "\nx = " << x <<
        ", y must be < " << x << "\n";

}
\$\endgroup\$
  • 1
    \$\begingroup\$ This is my first C++ program after 1 day writing in C++ . after 3 months total of java exp. \$\endgroup\$ – xyz Dec 10 '16 at 0:46
  • \$\begingroup\$ so if my format is bad, its my fault and i should have known better. \$\endgroup\$ – xyz Dec 10 '16 at 0:50
  • \$\begingroup\$ This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question. \$\endgroup\$ – Edward Dec 10 '16 at 0:53
4
\$\begingroup\$

using namespace std;

You import using namespace std;, but you actually only use it in your call to cout. As a general rule, you don't want to import this in its entirety, but to call the components directly, like in your calls to std::cin. See Why is using namespace std considered bad practice for more information.

Function Declaration

As a general rule, you order C++ functions from last used to first, unlike in Java. That means your oper() method would typically be written above your main() method. Typically, you only explicitly declare functions in header (.h) files, and when two functions call each other and you do not have a .h file to go with your .cpp file.

Comments

Please do not use comments when it is obvious what is happening. Your only comment here does not help understand either the how or why of the program.

Style

Please use braces around your if statements. At first, I thought you had a bug here because of the two lines, but then I realized it was just a line break in a single statement. Braces would help clear this up, as would writing the statement either on one line, or with better indentation as:

cout << "\nx = " << x
     << ", y must be < " << x << "\n";

Error Handling

Your program has absolutely no error handling, and that is perfectly understandable at this point. Eventually, however, you will need to use a try block to ensure your program will not crash if the user enters invalid input and a loop to ensure a valid value is eventually entered. Bjarne Stroustrup's "Programming: Principles and Practice Using C++" dedicates the majority of several chapters to proper error handling, and I strongly recommend you read this book; it will help you with your Java programming as well as your C++ programming.

Improvements

You only output the result if the x value entered is greater than the y value entered. Your program should output the result when y is greater than x, and when x is the same value as y as well.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ thank you for you input, its very straight forward :) i just dont understand the .h file part, i thought eclipse was pulling my leg always bringing one up every project. i spent hours reading about bits and bytes and i did not even check to read about the .h files. thanks for clearning up the two very important formatting questions i had over what goes first in a call function lol i seen it different in a few tutorials, and braces. thanks for clearing up a lot of my questions :) \$\endgroup\$ – xyz Dec 10 '16 at 1:03
  • \$\begingroup\$ thanks ill make those other improvements, and research error handling, and try statments. ill also try and get my hand on that book. i feel C++ is more natural IMO than java lol idk i really loved java and its structure, but C++ just flows. \$\endgroup\$ – xyz Dec 10 '16 at 1:06
  • 1
    \$\begingroup\$ wow ive grown since i've been here! \$\endgroup\$ – xyz Feb 12 '18 at 13:31
  • \$\begingroup\$ @SeanJ Congrats :) \$\endgroup\$ – user34073 Feb 12 '18 at 14:10
3
\$\begingroup\$

In addition to the main points already raised by @Hosch250, your code feels incomplete. You've declared oper as returning an int, however you neither return a value from the function, or use the returned value.

You may also want to consider your names for variables / methods. oper doesn't really tell the reader anything about what the method does.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ i have not figured out how to run it without it written that way. i spent a lot of my time learning form the runtime/compile errors, and did not think to look it up. but thank you for raising that point. \$\endgroup\$ – xyz Dec 10 '16 at 1:09
  • \$\begingroup\$ @SeanJ if you don't want to return a value, you can declare the method as void: void oper(). Alternately, you can use the value something like this: ideone.com/VxXHaQ \$\endgroup\$ – forsvarir Dec 10 '16 at 1:16
  • \$\begingroup\$ ohhhh wow thats very clear, thanks, im adding that to my brain ASAP once i review and study it for a bit. :) thanks so much \$\endgroup\$ – xyz Dec 10 '16 at 1:22

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.