Beginner C++ calculator

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP











up vote
8
down vote

favorite
4












I'm relatively new to programming, but am really enjoying it. I have taken a couple of classes but mostly code for fun (for now). I decided to make a program implementing some basic functions, switch statements, and user input. Please leave any feedback on how to make this program better.



I understand that I am not actually using the defined functions (add, subtract, etc.), but I put them in there with the intent of using them, I just don't know how.



#include <iostream>
using namespace std;

int add (int x, int y)
return x+y;
;

int divide (int x, int y)
return x/y;


int multiply (int x, int y)
return x*y;


int subtract (int x, int y)
return x-y;



int main()

int n1;
int n2;
int user14 = 0;

SomeLine:
cout << "Enter your 2 numbers: "<< endl;
cin >> n1;
cin >> n2;


cout << "Ok, now what do you want to do with those numbers? "<< endl;

cout << "1) Add: " << endl;
cout << "2) Divide: "<<endl;
cout << "3) Multiply: "<< endl;
cout << "4) Subtration: "<< endl;

cin >> user14;

switch (user14)

case 1:
cout << n1+n2 << endl;
break;
case 2:
cout << n1/n2 << endl;
break;
case 3:
cout << n1*n2<< endl;
break;
case 4:
cout << n1-n2 << endl;
break;



char userchoice;
cout << "Would you like to perform any other operations? y/n "<< endl;

cin >> userchoice;

if (userchoice=='y')
goto SomeLine;

else if(userchoice=='n')
goto Exit;


Exit:
return 0;

;









share|improve this question



















  • 1




    x/y; should make sure that y is not 0. Instead of using goto, a do.. while loop is better
    – Sailesh D
    Dec 6 at 6:50










  • Your program doesn't call the functions named multiply, divide, add and subtract. :)
    – Kaz
    Dec 6 at 14:30











  • Minor thing: If the code in the question doesn't look like (i.e. isn't indented the same as) your actual code, instead of manually indenting every line, try pasting the block in, selecting the entire block, and hitting Ctrl+K (Cmd+K on Mac).
    – Nic Hartley
    Dec 6 at 18:37














up vote
8
down vote

favorite
4












I'm relatively new to programming, but am really enjoying it. I have taken a couple of classes but mostly code for fun (for now). I decided to make a program implementing some basic functions, switch statements, and user input. Please leave any feedback on how to make this program better.



I understand that I am not actually using the defined functions (add, subtract, etc.), but I put them in there with the intent of using them, I just don't know how.



#include <iostream>
using namespace std;

int add (int x, int y)
return x+y;
;

int divide (int x, int y)
return x/y;


int multiply (int x, int y)
return x*y;


int subtract (int x, int y)
return x-y;



int main()

int n1;
int n2;
int user14 = 0;

SomeLine:
cout << "Enter your 2 numbers: "<< endl;
cin >> n1;
cin >> n2;


cout << "Ok, now what do you want to do with those numbers? "<< endl;

cout << "1) Add: " << endl;
cout << "2) Divide: "<<endl;
cout << "3) Multiply: "<< endl;
cout << "4) Subtration: "<< endl;

cin >> user14;

switch (user14)

case 1:
cout << n1+n2 << endl;
break;
case 2:
cout << n1/n2 << endl;
break;
case 3:
cout << n1*n2<< endl;
break;
case 4:
cout << n1-n2 << endl;
break;



char userchoice;
cout << "Would you like to perform any other operations? y/n "<< endl;

cin >> userchoice;

if (userchoice=='y')
goto SomeLine;

else if(userchoice=='n')
goto Exit;


Exit:
return 0;

;









share|improve this question



















  • 1




    x/y; should make sure that y is not 0. Instead of using goto, a do.. while loop is better
    – Sailesh D
    Dec 6 at 6:50










  • Your program doesn't call the functions named multiply, divide, add and subtract. :)
    – Kaz
    Dec 6 at 14:30











  • Minor thing: If the code in the question doesn't look like (i.e. isn't indented the same as) your actual code, instead of manually indenting every line, try pasting the block in, selecting the entire block, and hitting Ctrl+K (Cmd+K on Mac).
    – Nic Hartley
    Dec 6 at 18:37












up vote
8
down vote

favorite
4









up vote
8
down vote

favorite
4






4





I'm relatively new to programming, but am really enjoying it. I have taken a couple of classes but mostly code for fun (for now). I decided to make a program implementing some basic functions, switch statements, and user input. Please leave any feedback on how to make this program better.



I understand that I am not actually using the defined functions (add, subtract, etc.), but I put them in there with the intent of using them, I just don't know how.



#include <iostream>
using namespace std;

int add (int x, int y)
return x+y;
;

int divide (int x, int y)
return x/y;


int multiply (int x, int y)
return x*y;


int subtract (int x, int y)
return x-y;



int main()

int n1;
int n2;
int user14 = 0;

SomeLine:
cout << "Enter your 2 numbers: "<< endl;
cin >> n1;
cin >> n2;


cout << "Ok, now what do you want to do with those numbers? "<< endl;

cout << "1) Add: " << endl;
cout << "2) Divide: "<<endl;
cout << "3) Multiply: "<< endl;
cout << "4) Subtration: "<< endl;

cin >> user14;

switch (user14)

case 1:
cout << n1+n2 << endl;
break;
case 2:
cout << n1/n2 << endl;
break;
case 3:
cout << n1*n2<< endl;
break;
case 4:
cout << n1-n2 << endl;
break;



char userchoice;
cout << "Would you like to perform any other operations? y/n "<< endl;

cin >> userchoice;

if (userchoice=='y')
goto SomeLine;

else if(userchoice=='n')
goto Exit;


Exit:
return 0;

;









share|improve this question















I'm relatively new to programming, but am really enjoying it. I have taken a couple of classes but mostly code for fun (for now). I decided to make a program implementing some basic functions, switch statements, and user input. Please leave any feedback on how to make this program better.



I understand that I am not actually using the defined functions (add, subtract, etc.), but I put them in there with the intent of using them, I just don't know how.



#include <iostream>
using namespace std;

int add (int x, int y)
return x+y;
;

int divide (int x, int y)
return x/y;


int multiply (int x, int y)
return x*y;


int subtract (int x, int y)
return x-y;



int main()

int n1;
int n2;
int user14 = 0;

SomeLine:
cout << "Enter your 2 numbers: "<< endl;
cin >> n1;
cin >> n2;


cout << "Ok, now what do you want to do with those numbers? "<< endl;

cout << "1) Add: " << endl;
cout << "2) Divide: "<<endl;
cout << "3) Multiply: "<< endl;
cout << "4) Subtration: "<< endl;

cin >> user14;

switch (user14)

case 1:
cout << n1+n2 << endl;
break;
case 2:
cout << n1/n2 << endl;
break;
case 3:
cout << n1*n2<< endl;
break;
case 4:
cout << n1-n2 << endl;
break;



char userchoice;
cout << "Would you like to perform any other operations? y/n "<< endl;

cin >> userchoice;

if (userchoice=='y')
goto SomeLine;

else if(userchoice=='n')
goto Exit;


Exit:
return 0;

;






c++ calculator






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Dec 6 at 4:55









Jamal

30.2k11115226




30.2k11115226










asked Dec 6 at 4:48









braisedbeefcake

4412




4412







  • 1




    x/y; should make sure that y is not 0. Instead of using goto, a do.. while loop is better
    – Sailesh D
    Dec 6 at 6:50










  • Your program doesn't call the functions named multiply, divide, add and subtract. :)
    – Kaz
    Dec 6 at 14:30











  • Minor thing: If the code in the question doesn't look like (i.e. isn't indented the same as) your actual code, instead of manually indenting every line, try pasting the block in, selecting the entire block, and hitting Ctrl+K (Cmd+K on Mac).
    – Nic Hartley
    Dec 6 at 18:37












  • 1




    x/y; should make sure that y is not 0. Instead of using goto, a do.. while loop is better
    – Sailesh D
    Dec 6 at 6:50










  • Your program doesn't call the functions named multiply, divide, add and subtract. :)
    – Kaz
    Dec 6 at 14:30











  • Minor thing: If the code in the question doesn't look like (i.e. isn't indented the same as) your actual code, instead of manually indenting every line, try pasting the block in, selecting the entire block, and hitting Ctrl+K (Cmd+K on Mac).
    – Nic Hartley
    Dec 6 at 18:37







1




1




x/y; should make sure that y is not 0. Instead of using goto, a do.. while loop is better
– Sailesh D
Dec 6 at 6:50




x/y; should make sure that y is not 0. Instead of using goto, a do.. while loop is better
– Sailesh D
Dec 6 at 6:50












Your program doesn't call the functions named multiply, divide, add and subtract. :)
– Kaz
Dec 6 at 14:30





Your program doesn't call the functions named multiply, divide, add and subtract. :)
– Kaz
Dec 6 at 14:30













Minor thing: If the code in the question doesn't look like (i.e. isn't indented the same as) your actual code, instead of manually indenting every line, try pasting the block in, selecting the entire block, and hitting Ctrl+K (Cmd+K on Mac).
– Nic Hartley
Dec 6 at 18:37




Minor thing: If the code in the question doesn't look like (i.e. isn't indented the same as) your actual code, instead of manually indenting every line, try pasting the block in, selecting the entire block, and hitting Ctrl+K (Cmd+K on Mac).
– Nic Hartley
Dec 6 at 18:37










4 Answers
4






active

oldest

votes

















up vote
30
down vote













Welcome! Here is some feedback, in no particular order.



Avoid using namespace std



This advice is often repeated for good reason. See also: code guidelines SF.6. It's useful when you're learning the C++ standard library to know what belongs in the standard library and what doesn't.



cout << "Enter your 2 numbers: "<< endl;


Becomes



std::cout << "Enter your 2 numbers: "<< std::endl;


Consistent spacing



Maybe this is caused by the formatting on Stack Exchange. There are many online style guides, I would choose one that is most readable to you and your peers and stick with it throughout the source code.



int add (int x, int y)
return x+y;
;


Becomes



int add(int x, int y) 
return x + y;
;


Semicolons



Functions don't need semicolons at the end (main & add). This is only used for function prototypes.



int add(int x, int y) 
return x + y;
;


Becomes



int add(int x, int y) 
return x + y;



Avoid std::endl



Unless you need to flush the stream, using std::endl can be replaced with a newline character.



std::cout << "Enter your 2 numbers: "<< std::endl;


Becomes



std::cout << "Enter your 2 numbers:n";


Combine multiple std::cout and std::cin calls.



This is just my preference. It reduces the amount of code I have to write.



std::cin >> n1;
std::cin >> n2;


Becomes



std::cin >> n1 >> n2;


And



std::cout << "1) Add: " << std::endl;
std::cout << "2) Divide: "<< std::endl;
std::cout << "3) Multiply: "<< std::endl;
std::cout << "4) Subtration: "<< std::endl;


Becomes



std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";


In the above example, there are many other ways to do it. For example, you can use multi-line strings, adding the strings, and so on.



Make use of your functions



You define the add, divide, multiply and subtract functions, but you don't call them. Here is how you would do that:



std::cout << n1 + n2 << 'n';


Becomes



std::cout << add(n1, n2) << 'n';


It's as simple as that!



In complex applications, splitting things into functions makes the code easier to work with. For example, say you wanted the "add" operation to have saturating arithmetic (doesn't overflow). It's easier to add that logic to a separate function rather than inserting it into a long block of code.



Or, if you want to handle a division-by-zero case.



Don't use goto



While goto is useful in a few cases, it's not particularly useful here. Instead you can use a do-while loop.



SomeLine:

// code

if (userchoice == 'y')
goto SomeLine;

else if(userchoice == 'n')
goto Exit;


Exit:
return 0;


Becomes



do 
// code
while (userchoice == 'y');

return 0;


Variable naming



The variables x and y are used in the function signatures. You can also use those names in main. I'd prefer that over n1 and n2. Likewise, user14 is vague (do we change it to user15 if we include another operation?). Something like operation or op isn't vague. userchoice is a bit long, exit is shorter and clearly shows its purpose.



const



Make use of the const keyword where it makes sense. For example, in add, we intend for the x and y variables to never themselves be modified. So we can declare them as const, and the compiler will give us an error if we accidentally change their values.



This isn't particularly useful anything other than readability here, but can be useful if you get into more advanced topics, such as const references.



The functions add, divide, multiply, and subtract can be constexpr. The compiler may already evaluate these at compile time, but using constexpr marks them as such (broadly speaking).



Move variables to the lowest scope



Now that we've gotten rid of the gotos, we can move x, y, and op to a lower scope. They can also all be declared on the same line.



Consider edge cases



The following are a few example cases where you may wish to add extra handling -- or leave as it currently is.



  • What if the user enters a letter where you're expecting an integer?

  • What if the user enters an operation number outside of the range?

  • What about possible signed integer overflow (undefined behavior)?

  • What about division-by-zero?

  • What if the input stream is no longer valid after user input?

  • ...

Conclusion



Here is the code I ended up with:



#include <iostream>

constexpr int add(const int x, const int y)
return x + y;


constexpr int divide(const int x, const int y)
return x / y;


constexpr int multiply(const int x, const int y)
return x * y;


constexpr int subtract(const int x, const int y)
return x - y;



int main()
char exit;

do
int x, y, op;

std::cout << "Enter your 2 numbers:n";

std::cin >> x >> y;

std::cout << "Ok, now what do you want to do with those numbers?n";
std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";

std::cin >> op;

switch (op)
case 1:
std::cout << add(x, y) << 'n';
break;
case 2:
std::cout << divide(x, y) << 'n';
break;
case 3:
std::cout << multiply(x, y) << 'n';
break;
case 4:
std::cout << subtract(x, y) << 'n';
break;


std::cout << "Would you like to perform any other operations? y/nn";

std::cin >> exit;
while (exit == 'y');

return 0;



Hopefully this has been helpful. I hope you continue to enjoy programming as much as we all do!






share|improve this answer


















  • 1




    Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
    – braisedbeefcake
    Dec 6 at 8:06










  • Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
    – Toby Speight
    Dec 6 at 9:52






  • 1




    Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
    – Sembei Norimaki
    Dec 6 at 12:23







  • 1




    It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
    – Pete Kirkham
    Dec 6 at 12:39






  • 1




    @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
    – Nic Hartley
    Dec 6 at 18:39


















up vote
9
down vote













One item missing from the other replies is that you should avoid mixing output and computations.



Instead of



 switch (op) 
case 1:
std::cout << add(x, y) << 'n';
break;
case 2:
std::cout << divide(x, y) << 'n';
break;
case 3:
std::cout << multiply(x, y) << 'n';
break;
case 4:
std::cout << subtract(x, y) << 'n';
break;



Use:



 int res;
switch (op)
case 1:
res = add(x, y);
break;
case 2:
res = divide(x, y);
break;
case 3:
res = multiply(x, y);
break;
case 4:
res = divide(x, y);
break;

std::cout << res << 'n';


Or turn this into a separate function:



 int computation(int op, int x, y) 
switch (op)
case 1:
return add(x, y);
case 2:
return divide(x, y);
case 3:
return multiply(x, y);
case 4:
return divide(x, y);


...
int res=computation(op, x, y);


There are two reasons:



  • Easier to debug.

  • Easier to change the formatting of the output in a common way.





share|improve this answer





























    up vote
    4
    down vote













    Welcome on Code Review!




    Don't use using namespace std;



    Putting using namespace std at the top of every program is a bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.



    • Although it's "safe" to use it in some place (e.g. implementation files), as long as you aren't familiar with the features of c ++, try to avoid it.

    • It led to a world of name collisions. (best case)

    • It's source of silent errors and weird bugs. (worst case)

    • If typing std:: is so tedious for you, try to import only truncated namespaces. ( e.g. individual identifiers like using std::string; or nested names like using namespace std::literals).

    • If importing nested namespaces still too awful for you, try to do it inside a restricted scope (e.g. a functions) and not in global scope.


    Ensure validity of parameters



    In your add(), substract(), multiply() and divide() function, you don't check the validity of given inputs. You could surely use assertions to ensure that the operation don't lead to integer overflow/underflow or division by zero.



    You could use assertions to check preconditions, postconditions and invariants. It make your code more explicit and you avoid possibly broke you code when modifying.




    • How to use assertions in C (for C, but still applicable here)


    • How and When to Use C's assert() Macro (same, also valid)


    • Why should I use asserts?


    Inputs / Outputs



    • When you read values from user input, consider it ill-formed, so check for validity and sanitize input. Never trust user, They all try to broke your program.

    • Using std::endl sending a 'n' and then flushes the output buffer. So std::endl is more expensive in performance. So, use 'n' 'n' and then, if you want to manually flush the stream, explicitly call std::flush.


    Use your functions



    If you don't know how to use your function, I think you really have to learn to basics. Their are a lot of resources on the net. Once you get basics, dive into the C++ FAQ and the C++ Core Guideline to get some good practices.



    Also, it's Christmas soon, a good opportunity to ask for a good book about C++




    Misc



    • When it's possible, try to avoid using goto.


    • Don't return 0 from main().

    • You should also fix indentation.





    share|improve this answer






















    • Thank you so much for your feedback!
      – braisedbeefcake
      Dec 6 at 8:08










    • namespace std::string; is clearly an error, since std::string names a class, not a namespace.
      – Toby Speight
      Dec 6 at 9:55







    • 2




      return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
      – Jack Aidley
      Dec 6 at 10:27










    • @TobySpeight thx for pointing out this typo.
      – Calak
      Dec 6 at 11:48






    • 2




      @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
      – Martin York
      Dec 6 at 18:18

















    up vote
    2
    down vote













    Default case!



    In addition to the other excellent answers, you could add a default case to your switch statement! This is the case that is called when something other than the expected input is received.



    In your example, it would end up looking something like this:



    switch (user14)

    case 1:
    Add(n1, n2);
    break;
    case 2:
    Subtract(n1, n2);
    break;
    case 3:
    Multiply(n1, n2);
    break;
    case 4:
    Divide(n1, n2);
    break;
    Default:
    cout << "Invalid operationn";
    break;



    In the above code, a 1, 2, 3, or 4 will perform the expected corresponding action, as in your original code. But in your original code, any other numerical input will just exit the switch statement and move on to the next thing, without telling the user that no action was taken. In the example above, it will instead tell the user why no action is being taken.



    It's a useful way to let the user know that their input was invalid, but it can also be used to automatically take an action if none of the cases are matched. If that is confusing, you can think of default like the final "else" at the end of a series of "if/else if" statements.



    On the other hand, if there's no need for user feedback and there's no "default" action that you want to happen normally, then there's no need to make a default case.



    Another change I might suggest is for user14, I would personally use a char instead of an int. This will stop the program from crashing if the user inputs something other than an integer. But then, if you know that the input will only ever be an integer, then there's really no need to take that precaution!



    Happy coding!






    share|improve this answer




















      Your Answer





      StackExchange.ifUsing("editor", function ()
      return StackExchange.using("mathjaxEditing", function ()
      StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
      StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
      );
      );
      , "mathjax-editing");

      StackExchange.ifUsing("editor", function ()
      StackExchange.using("externalEditor", function ()
      StackExchange.using("snippets", function ()
      StackExchange.snippets.init();
      );
      );
      , "code-snippets");

      StackExchange.ready(function()
      var channelOptions =
      tags: "".split(" "),
      id: "196"
      ;
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function()
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled)
      StackExchange.using("snippets", function()
      createEditor();
      );

      else
      createEditor();

      );

      function createEditor()
      StackExchange.prepareEditor(
      heartbeatType: 'answer',
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader:
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      ,
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      );



      );













      draft saved

      draft discarded


















      StackExchange.ready(
      function ()
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209130%2fbeginner-c-calculator%23new-answer', 'question_page');

      );

      Post as a guest















      Required, but never shown

























      4 Answers
      4






      active

      oldest

      votes








      4 Answers
      4






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes








      up vote
      30
      down vote













      Welcome! Here is some feedback, in no particular order.



      Avoid using namespace std



      This advice is often repeated for good reason. See also: code guidelines SF.6. It's useful when you're learning the C++ standard library to know what belongs in the standard library and what doesn't.



      cout << "Enter your 2 numbers: "<< endl;


      Becomes



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Consistent spacing



      Maybe this is caused by the formatting on Stack Exchange. There are many online style guides, I would choose one that is most readable to you and your peers and stick with it throughout the source code.



      int add (int x, int y)
      return x+y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;
      ;


      Semicolons



      Functions don't need semicolons at the end (main & add). This is only used for function prototypes.



      int add(int x, int y) 
      return x + y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;



      Avoid std::endl



      Unless you need to flush the stream, using std::endl can be replaced with a newline character.



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Becomes



      std::cout << "Enter your 2 numbers:n";


      Combine multiple std::cout and std::cin calls.



      This is just my preference. It reduces the amount of code I have to write.



      std::cin >> n1;
      std::cin >> n2;


      Becomes



      std::cin >> n1 >> n2;


      And



      std::cout << "1) Add: " << std::endl;
      std::cout << "2) Divide: "<< std::endl;
      std::cout << "3) Multiply: "<< std::endl;
      std::cout << "4) Subtration: "<< std::endl;


      Becomes



      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";


      In the above example, there are many other ways to do it. For example, you can use multi-line strings, adding the strings, and so on.



      Make use of your functions



      You define the add, divide, multiply and subtract functions, but you don't call them. Here is how you would do that:



      std::cout << n1 + n2 << 'n';


      Becomes



      std::cout << add(n1, n2) << 'n';


      It's as simple as that!



      In complex applications, splitting things into functions makes the code easier to work with. For example, say you wanted the "add" operation to have saturating arithmetic (doesn't overflow). It's easier to add that logic to a separate function rather than inserting it into a long block of code.



      Or, if you want to handle a division-by-zero case.



      Don't use goto



      While goto is useful in a few cases, it's not particularly useful here. Instead you can use a do-while loop.



      SomeLine:

      // code

      if (userchoice == 'y')
      goto SomeLine;

      else if(userchoice == 'n')
      goto Exit;


      Exit:
      return 0;


      Becomes



      do 
      // code
      while (userchoice == 'y');

      return 0;


      Variable naming



      The variables x and y are used in the function signatures. You can also use those names in main. I'd prefer that over n1 and n2. Likewise, user14 is vague (do we change it to user15 if we include another operation?). Something like operation or op isn't vague. userchoice is a bit long, exit is shorter and clearly shows its purpose.



      const



      Make use of the const keyword where it makes sense. For example, in add, we intend for the x and y variables to never themselves be modified. So we can declare them as const, and the compiler will give us an error if we accidentally change their values.



      This isn't particularly useful anything other than readability here, but can be useful if you get into more advanced topics, such as const references.



      The functions add, divide, multiply, and subtract can be constexpr. The compiler may already evaluate these at compile time, but using constexpr marks them as such (broadly speaking).



      Move variables to the lowest scope



      Now that we've gotten rid of the gotos, we can move x, y, and op to a lower scope. They can also all be declared on the same line.



      Consider edge cases



      The following are a few example cases where you may wish to add extra handling -- or leave as it currently is.



      • What if the user enters a letter where you're expecting an integer?

      • What if the user enters an operation number outside of the range?

      • What about possible signed integer overflow (undefined behavior)?

      • What about division-by-zero?

      • What if the input stream is no longer valid after user input?

      • ...

      Conclusion



      Here is the code I ended up with:



      #include <iostream>

      constexpr int add(const int x, const int y)
      return x + y;


      constexpr int divide(const int x, const int y)
      return x / y;


      constexpr int multiply(const int x, const int y)
      return x * y;


      constexpr int subtract(const int x, const int y)
      return x - y;



      int main()
      char exit;

      do
      int x, y, op;

      std::cout << "Enter your 2 numbers:n";

      std::cin >> x >> y;

      std::cout << "Ok, now what do you want to do with those numbers?n";
      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";

      std::cin >> op;

      switch (op)
      case 1:
      std::cout << add(x, y) << 'n';
      break;
      case 2:
      std::cout << divide(x, y) << 'n';
      break;
      case 3:
      std::cout << multiply(x, y) << 'n';
      break;
      case 4:
      std::cout << subtract(x, y) << 'n';
      break;


      std::cout << "Would you like to perform any other operations? y/nn";

      std::cin >> exit;
      while (exit == 'y');

      return 0;



      Hopefully this has been helpful. I hope you continue to enjoy programming as much as we all do!






      share|improve this answer


















      • 1




        Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
        – braisedbeefcake
        Dec 6 at 8:06










      • Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
        – Toby Speight
        Dec 6 at 9:52






      • 1




        Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
        – Sembei Norimaki
        Dec 6 at 12:23







      • 1




        It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
        – Pete Kirkham
        Dec 6 at 12:39






      • 1




        @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
        – Nic Hartley
        Dec 6 at 18:39















      up vote
      30
      down vote













      Welcome! Here is some feedback, in no particular order.



      Avoid using namespace std



      This advice is often repeated for good reason. See also: code guidelines SF.6. It's useful when you're learning the C++ standard library to know what belongs in the standard library and what doesn't.



      cout << "Enter your 2 numbers: "<< endl;


      Becomes



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Consistent spacing



      Maybe this is caused by the formatting on Stack Exchange. There are many online style guides, I would choose one that is most readable to you and your peers and stick with it throughout the source code.



      int add (int x, int y)
      return x+y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;
      ;


      Semicolons



      Functions don't need semicolons at the end (main & add). This is only used for function prototypes.



      int add(int x, int y) 
      return x + y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;



      Avoid std::endl



      Unless you need to flush the stream, using std::endl can be replaced with a newline character.



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Becomes



      std::cout << "Enter your 2 numbers:n";


      Combine multiple std::cout and std::cin calls.



      This is just my preference. It reduces the amount of code I have to write.



      std::cin >> n1;
      std::cin >> n2;


      Becomes



      std::cin >> n1 >> n2;


      And



      std::cout << "1) Add: " << std::endl;
      std::cout << "2) Divide: "<< std::endl;
      std::cout << "3) Multiply: "<< std::endl;
      std::cout << "4) Subtration: "<< std::endl;


      Becomes



      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";


      In the above example, there are many other ways to do it. For example, you can use multi-line strings, adding the strings, and so on.



      Make use of your functions



      You define the add, divide, multiply and subtract functions, but you don't call them. Here is how you would do that:



      std::cout << n1 + n2 << 'n';


      Becomes



      std::cout << add(n1, n2) << 'n';


      It's as simple as that!



      In complex applications, splitting things into functions makes the code easier to work with. For example, say you wanted the "add" operation to have saturating arithmetic (doesn't overflow). It's easier to add that logic to a separate function rather than inserting it into a long block of code.



      Or, if you want to handle a division-by-zero case.



      Don't use goto



      While goto is useful in a few cases, it's not particularly useful here. Instead you can use a do-while loop.



      SomeLine:

      // code

      if (userchoice == 'y')
      goto SomeLine;

      else if(userchoice == 'n')
      goto Exit;


      Exit:
      return 0;


      Becomes



      do 
      // code
      while (userchoice == 'y');

      return 0;


      Variable naming



      The variables x and y are used in the function signatures. You can also use those names in main. I'd prefer that over n1 and n2. Likewise, user14 is vague (do we change it to user15 if we include another operation?). Something like operation or op isn't vague. userchoice is a bit long, exit is shorter and clearly shows its purpose.



      const



      Make use of the const keyword where it makes sense. For example, in add, we intend for the x and y variables to never themselves be modified. So we can declare them as const, and the compiler will give us an error if we accidentally change their values.



      This isn't particularly useful anything other than readability here, but can be useful if you get into more advanced topics, such as const references.



      The functions add, divide, multiply, and subtract can be constexpr. The compiler may already evaluate these at compile time, but using constexpr marks them as such (broadly speaking).



      Move variables to the lowest scope



      Now that we've gotten rid of the gotos, we can move x, y, and op to a lower scope. They can also all be declared on the same line.



      Consider edge cases



      The following are a few example cases where you may wish to add extra handling -- or leave as it currently is.



      • What if the user enters a letter where you're expecting an integer?

      • What if the user enters an operation number outside of the range?

      • What about possible signed integer overflow (undefined behavior)?

      • What about division-by-zero?

      • What if the input stream is no longer valid after user input?

      • ...

      Conclusion



      Here is the code I ended up with:



      #include <iostream>

      constexpr int add(const int x, const int y)
      return x + y;


      constexpr int divide(const int x, const int y)
      return x / y;


      constexpr int multiply(const int x, const int y)
      return x * y;


      constexpr int subtract(const int x, const int y)
      return x - y;



      int main()
      char exit;

      do
      int x, y, op;

      std::cout << "Enter your 2 numbers:n";

      std::cin >> x >> y;

      std::cout << "Ok, now what do you want to do with those numbers?n";
      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";

      std::cin >> op;

      switch (op)
      case 1:
      std::cout << add(x, y) << 'n';
      break;
      case 2:
      std::cout << divide(x, y) << 'n';
      break;
      case 3:
      std::cout << multiply(x, y) << 'n';
      break;
      case 4:
      std::cout << subtract(x, y) << 'n';
      break;


      std::cout << "Would you like to perform any other operations? y/nn";

      std::cin >> exit;
      while (exit == 'y');

      return 0;



      Hopefully this has been helpful. I hope you continue to enjoy programming as much as we all do!






      share|improve this answer


















      • 1




        Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
        – braisedbeefcake
        Dec 6 at 8:06










      • Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
        – Toby Speight
        Dec 6 at 9:52






      • 1




        Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
        – Sembei Norimaki
        Dec 6 at 12:23







      • 1




        It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
        – Pete Kirkham
        Dec 6 at 12:39






      • 1




        @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
        – Nic Hartley
        Dec 6 at 18:39













      up vote
      30
      down vote










      up vote
      30
      down vote









      Welcome! Here is some feedback, in no particular order.



      Avoid using namespace std



      This advice is often repeated for good reason. See also: code guidelines SF.6. It's useful when you're learning the C++ standard library to know what belongs in the standard library and what doesn't.



      cout << "Enter your 2 numbers: "<< endl;


      Becomes



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Consistent spacing



      Maybe this is caused by the formatting on Stack Exchange. There are many online style guides, I would choose one that is most readable to you and your peers and stick with it throughout the source code.



      int add (int x, int y)
      return x+y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;
      ;


      Semicolons



      Functions don't need semicolons at the end (main & add). This is only used for function prototypes.



      int add(int x, int y) 
      return x + y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;



      Avoid std::endl



      Unless you need to flush the stream, using std::endl can be replaced with a newline character.



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Becomes



      std::cout << "Enter your 2 numbers:n";


      Combine multiple std::cout and std::cin calls.



      This is just my preference. It reduces the amount of code I have to write.



      std::cin >> n1;
      std::cin >> n2;


      Becomes



      std::cin >> n1 >> n2;


      And



      std::cout << "1) Add: " << std::endl;
      std::cout << "2) Divide: "<< std::endl;
      std::cout << "3) Multiply: "<< std::endl;
      std::cout << "4) Subtration: "<< std::endl;


      Becomes



      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";


      In the above example, there are many other ways to do it. For example, you can use multi-line strings, adding the strings, and so on.



      Make use of your functions



      You define the add, divide, multiply and subtract functions, but you don't call them. Here is how you would do that:



      std::cout << n1 + n2 << 'n';


      Becomes



      std::cout << add(n1, n2) << 'n';


      It's as simple as that!



      In complex applications, splitting things into functions makes the code easier to work with. For example, say you wanted the "add" operation to have saturating arithmetic (doesn't overflow). It's easier to add that logic to a separate function rather than inserting it into a long block of code.



      Or, if you want to handle a division-by-zero case.



      Don't use goto



      While goto is useful in a few cases, it's not particularly useful here. Instead you can use a do-while loop.



      SomeLine:

      // code

      if (userchoice == 'y')
      goto SomeLine;

      else if(userchoice == 'n')
      goto Exit;


      Exit:
      return 0;


      Becomes



      do 
      // code
      while (userchoice == 'y');

      return 0;


      Variable naming



      The variables x and y are used in the function signatures. You can also use those names in main. I'd prefer that over n1 and n2. Likewise, user14 is vague (do we change it to user15 if we include another operation?). Something like operation or op isn't vague. userchoice is a bit long, exit is shorter and clearly shows its purpose.



      const



      Make use of the const keyword where it makes sense. For example, in add, we intend for the x and y variables to never themselves be modified. So we can declare them as const, and the compiler will give us an error if we accidentally change their values.



      This isn't particularly useful anything other than readability here, but can be useful if you get into more advanced topics, such as const references.



      The functions add, divide, multiply, and subtract can be constexpr. The compiler may already evaluate these at compile time, but using constexpr marks them as such (broadly speaking).



      Move variables to the lowest scope



      Now that we've gotten rid of the gotos, we can move x, y, and op to a lower scope. They can also all be declared on the same line.



      Consider edge cases



      The following are a few example cases where you may wish to add extra handling -- or leave as it currently is.



      • What if the user enters a letter where you're expecting an integer?

      • What if the user enters an operation number outside of the range?

      • What about possible signed integer overflow (undefined behavior)?

      • What about division-by-zero?

      • What if the input stream is no longer valid after user input?

      • ...

      Conclusion



      Here is the code I ended up with:



      #include <iostream>

      constexpr int add(const int x, const int y)
      return x + y;


      constexpr int divide(const int x, const int y)
      return x / y;


      constexpr int multiply(const int x, const int y)
      return x * y;


      constexpr int subtract(const int x, const int y)
      return x - y;



      int main()
      char exit;

      do
      int x, y, op;

      std::cout << "Enter your 2 numbers:n";

      std::cin >> x >> y;

      std::cout << "Ok, now what do you want to do with those numbers?n";
      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";

      std::cin >> op;

      switch (op)
      case 1:
      std::cout << add(x, y) << 'n';
      break;
      case 2:
      std::cout << divide(x, y) << 'n';
      break;
      case 3:
      std::cout << multiply(x, y) << 'n';
      break;
      case 4:
      std::cout << subtract(x, y) << 'n';
      break;


      std::cout << "Would you like to perform any other operations? y/nn";

      std::cin >> exit;
      while (exit == 'y');

      return 0;



      Hopefully this has been helpful. I hope you continue to enjoy programming as much as we all do!






      share|improve this answer














      Welcome! Here is some feedback, in no particular order.



      Avoid using namespace std



      This advice is often repeated for good reason. See also: code guidelines SF.6. It's useful when you're learning the C++ standard library to know what belongs in the standard library and what doesn't.



      cout << "Enter your 2 numbers: "<< endl;


      Becomes



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Consistent spacing



      Maybe this is caused by the formatting on Stack Exchange. There are many online style guides, I would choose one that is most readable to you and your peers and stick with it throughout the source code.



      int add (int x, int y)
      return x+y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;
      ;


      Semicolons



      Functions don't need semicolons at the end (main & add). This is only used for function prototypes.



      int add(int x, int y) 
      return x + y;
      ;


      Becomes



      int add(int x, int y) 
      return x + y;



      Avoid std::endl



      Unless you need to flush the stream, using std::endl can be replaced with a newline character.



      std::cout << "Enter your 2 numbers: "<< std::endl;


      Becomes



      std::cout << "Enter your 2 numbers:n";


      Combine multiple std::cout and std::cin calls.



      This is just my preference. It reduces the amount of code I have to write.



      std::cin >> n1;
      std::cin >> n2;


      Becomes



      std::cin >> n1 >> n2;


      And



      std::cout << "1) Add: " << std::endl;
      std::cout << "2) Divide: "<< std::endl;
      std::cout << "3) Multiply: "<< std::endl;
      std::cout << "4) Subtration: "<< std::endl;


      Becomes



      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";


      In the above example, there are many other ways to do it. For example, you can use multi-line strings, adding the strings, and so on.



      Make use of your functions



      You define the add, divide, multiply and subtract functions, but you don't call them. Here is how you would do that:



      std::cout << n1 + n2 << 'n';


      Becomes



      std::cout << add(n1, n2) << 'n';


      It's as simple as that!



      In complex applications, splitting things into functions makes the code easier to work with. For example, say you wanted the "add" operation to have saturating arithmetic (doesn't overflow). It's easier to add that logic to a separate function rather than inserting it into a long block of code.



      Or, if you want to handle a division-by-zero case.



      Don't use goto



      While goto is useful in a few cases, it's not particularly useful here. Instead you can use a do-while loop.



      SomeLine:

      // code

      if (userchoice == 'y')
      goto SomeLine;

      else if(userchoice == 'n')
      goto Exit;


      Exit:
      return 0;


      Becomes



      do 
      // code
      while (userchoice == 'y');

      return 0;


      Variable naming



      The variables x and y are used in the function signatures. You can also use those names in main. I'd prefer that over n1 and n2. Likewise, user14 is vague (do we change it to user15 if we include another operation?). Something like operation or op isn't vague. userchoice is a bit long, exit is shorter and clearly shows its purpose.



      const



      Make use of the const keyword where it makes sense. For example, in add, we intend for the x and y variables to never themselves be modified. So we can declare them as const, and the compiler will give us an error if we accidentally change their values.



      This isn't particularly useful anything other than readability here, but can be useful if you get into more advanced topics, such as const references.



      The functions add, divide, multiply, and subtract can be constexpr. The compiler may already evaluate these at compile time, but using constexpr marks them as such (broadly speaking).



      Move variables to the lowest scope



      Now that we've gotten rid of the gotos, we can move x, y, and op to a lower scope. They can also all be declared on the same line.



      Consider edge cases



      The following are a few example cases where you may wish to add extra handling -- or leave as it currently is.



      • What if the user enters a letter where you're expecting an integer?

      • What if the user enters an operation number outside of the range?

      • What about possible signed integer overflow (undefined behavior)?

      • What about division-by-zero?

      • What if the input stream is no longer valid after user input?

      • ...

      Conclusion



      Here is the code I ended up with:



      #include <iostream>

      constexpr int add(const int x, const int y)
      return x + y;


      constexpr int divide(const int x, const int y)
      return x / y;


      constexpr int multiply(const int x, const int y)
      return x * y;


      constexpr int subtract(const int x, const int y)
      return x - y;



      int main()
      char exit;

      do
      int x, y, op;

      std::cout << "Enter your 2 numbers:n";

      std::cin >> x >> y;

      std::cout << "Ok, now what do you want to do with those numbers?n";
      std::cout << "1) Add:n2) Divide:n3) Multiply:n4) Subtration:n";

      std::cin >> op;

      switch (op)
      case 1:
      std::cout << add(x, y) << 'n';
      break;
      case 2:
      std::cout << divide(x, y) << 'n';
      break;
      case 3:
      std::cout << multiply(x, y) << 'n';
      break;
      case 4:
      std::cout << subtract(x, y) << 'n';
      break;


      std::cout << "Would you like to perform any other operations? y/nn";

      std::cin >> exit;
      while (exit == 'y');

      return 0;



      Hopefully this has been helpful. I hope you continue to enjoy programming as much as we all do!







      share|improve this answer














      share|improve this answer



      share|improve this answer








      edited Dec 6 at 13:43

























      answered Dec 6 at 6:56









      esote

      1,7251933




      1,7251933







      • 1




        Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
        – braisedbeefcake
        Dec 6 at 8:06










      • Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
        – Toby Speight
        Dec 6 at 9:52






      • 1




        Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
        – Sembei Norimaki
        Dec 6 at 12:23







      • 1




        It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
        – Pete Kirkham
        Dec 6 at 12:39






      • 1




        @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
        – Nic Hartley
        Dec 6 at 18:39













      • 1




        Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
        – braisedbeefcake
        Dec 6 at 8:06










      • Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
        – Toby Speight
        Dec 6 at 9:52






      • 1




        Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
        – Sembei Norimaki
        Dec 6 at 12:23







      • 1




        It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
        – Pete Kirkham
        Dec 6 at 12:39






      • 1




        @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
        – Nic Hartley
        Dec 6 at 18:39








      1




      1




      Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
      – braisedbeefcake
      Dec 6 at 8:06




      Wow! Such great feedback. The indenting was due to formatting, like you said. I will get in the habit of avoiding using namespace std. I’ve taken 3 coding classes (c++ 1 and 2, and data structures) and all my instructors have used it. Poor practice I suppose. I am familiar with do while loops and am not entirely sure why I chose otherwise. Getting in the habit of declaring variables in single lines seems to be very effective as well. I look forward to posting more of my programs as I progress and hopefully return as much useful feedback as I have. Thank you!
      – braisedbeefcake
      Dec 6 at 8:06












      Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
      – Toby Speight
      Dec 6 at 9:52




      Minor nit: add() is a function, not a "method" (by which I assume you mean a member function, since C++ doesn't define the term).
      – Toby Speight
      Dec 6 at 9:52




      1




      1




      Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
      – Sembei Norimaki
      Dec 6 at 12:23





      Could you elaborate on your point about avoiding std::endl and instead using n? Is it just to type less or there are important differences to be taken in account? Thanks
      – Sembei Norimaki
      Dec 6 at 12:23





      1




      1




      It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
      – Pete Kirkham
      Dec 6 at 12:39




      It's not a header file, so your using namespace std guideline does not apply. I'd tend to go the other way on multiple calls to std::cout << as merging them onto one line is harder to maintain and harder to read and spot the spelling error. Putting constexpr on a function that is only used in one file to process user input is probably YAGNI.
      – Pete Kirkham
      Dec 6 at 12:39




      1




      1




      @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
      – Nic Hartley
      Dec 6 at 18:39





      @PeteKirkham It's still going to bite you in the ass if you get used to doing it. Dive into ODR and ADL stuff -- it's "fun". It's not just about not using namespace std, but always qualifying calls.
      – Nic Hartley
      Dec 6 at 18:39













      up vote
      9
      down vote













      One item missing from the other replies is that you should avoid mixing output and computations.



      Instead of



       switch (op) 
      case 1:
      std::cout << add(x, y) << 'n';
      break;
      case 2:
      std::cout << divide(x, y) << 'n';
      break;
      case 3:
      std::cout << multiply(x, y) << 'n';
      break;
      case 4:
      std::cout << subtract(x, y) << 'n';
      break;



      Use:



       int res;
      switch (op)
      case 1:
      res = add(x, y);
      break;
      case 2:
      res = divide(x, y);
      break;
      case 3:
      res = multiply(x, y);
      break;
      case 4:
      res = divide(x, y);
      break;

      std::cout << res << 'n';


      Or turn this into a separate function:



       int computation(int op, int x, y) 
      switch (op)
      case 1:
      return add(x, y);
      case 2:
      return divide(x, y);
      case 3:
      return multiply(x, y);
      case 4:
      return divide(x, y);


      ...
      int res=computation(op, x, y);


      There are two reasons:



      • Easier to debug.

      • Easier to change the formatting of the output in a common way.





      share|improve this answer


























        up vote
        9
        down vote













        One item missing from the other replies is that you should avoid mixing output and computations.



        Instead of



         switch (op) 
        case 1:
        std::cout << add(x, y) << 'n';
        break;
        case 2:
        std::cout << divide(x, y) << 'n';
        break;
        case 3:
        std::cout << multiply(x, y) << 'n';
        break;
        case 4:
        std::cout << subtract(x, y) << 'n';
        break;



        Use:



         int res;
        switch (op)
        case 1:
        res = add(x, y);
        break;
        case 2:
        res = divide(x, y);
        break;
        case 3:
        res = multiply(x, y);
        break;
        case 4:
        res = divide(x, y);
        break;

        std::cout << res << 'n';


        Or turn this into a separate function:



         int computation(int op, int x, y) 
        switch (op)
        case 1:
        return add(x, y);
        case 2:
        return divide(x, y);
        case 3:
        return multiply(x, y);
        case 4:
        return divide(x, y);


        ...
        int res=computation(op, x, y);


        There are two reasons:



        • Easier to debug.

        • Easier to change the formatting of the output in a common way.





        share|improve this answer
























          up vote
          9
          down vote










          up vote
          9
          down vote









          One item missing from the other replies is that you should avoid mixing output and computations.



          Instead of



           switch (op) 
          case 1:
          std::cout << add(x, y) << 'n';
          break;
          case 2:
          std::cout << divide(x, y) << 'n';
          break;
          case 3:
          std::cout << multiply(x, y) << 'n';
          break;
          case 4:
          std::cout << subtract(x, y) << 'n';
          break;



          Use:



           int res;
          switch (op)
          case 1:
          res = add(x, y);
          break;
          case 2:
          res = divide(x, y);
          break;
          case 3:
          res = multiply(x, y);
          break;
          case 4:
          res = divide(x, y);
          break;

          std::cout << res << 'n';


          Or turn this into a separate function:



           int computation(int op, int x, y) 
          switch (op)
          case 1:
          return add(x, y);
          case 2:
          return divide(x, y);
          case 3:
          return multiply(x, y);
          case 4:
          return divide(x, y);


          ...
          int res=computation(op, x, y);


          There are two reasons:



          • Easier to debug.

          • Easier to change the formatting of the output in a common way.





          share|improve this answer














          One item missing from the other replies is that you should avoid mixing output and computations.



          Instead of



           switch (op) 
          case 1:
          std::cout << add(x, y) << 'n';
          break;
          case 2:
          std::cout << divide(x, y) << 'n';
          break;
          case 3:
          std::cout << multiply(x, y) << 'n';
          break;
          case 4:
          std::cout << subtract(x, y) << 'n';
          break;



          Use:



           int res;
          switch (op)
          case 1:
          res = add(x, y);
          break;
          case 2:
          res = divide(x, y);
          break;
          case 3:
          res = multiply(x, y);
          break;
          case 4:
          res = divide(x, y);
          break;

          std::cout << res << 'n';


          Or turn this into a separate function:



           int computation(int op, int x, y) 
          switch (op)
          case 1:
          return add(x, y);
          case 2:
          return divide(x, y);
          case 3:
          return multiply(x, y);
          case 4:
          return divide(x, y);


          ...
          int res=computation(op, x, y);


          There are two reasons:



          • Easier to debug.

          • Easier to change the formatting of the output in a common way.






          share|improve this answer














          share|improve this answer



          share|improve this answer








          edited Dec 6 at 15:09

























          answered Dec 6 at 14:33









          Hans Olsson

          2613




          2613




















              up vote
              4
              down vote













              Welcome on Code Review!




              Don't use using namespace std;



              Putting using namespace std at the top of every program is a bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.



              • Although it's "safe" to use it in some place (e.g. implementation files), as long as you aren't familiar with the features of c ++, try to avoid it.

              • It led to a world of name collisions. (best case)

              • It's source of silent errors and weird bugs. (worst case)

              • If typing std:: is so tedious for you, try to import only truncated namespaces. ( e.g. individual identifiers like using std::string; or nested names like using namespace std::literals).

              • If importing nested namespaces still too awful for you, try to do it inside a restricted scope (e.g. a functions) and not in global scope.


              Ensure validity of parameters



              In your add(), substract(), multiply() and divide() function, you don't check the validity of given inputs. You could surely use assertions to ensure that the operation don't lead to integer overflow/underflow or division by zero.



              You could use assertions to check preconditions, postconditions and invariants. It make your code more explicit and you avoid possibly broke you code when modifying.




              • How to use assertions in C (for C, but still applicable here)


              • How and When to Use C's assert() Macro (same, also valid)


              • Why should I use asserts?


              Inputs / Outputs



              • When you read values from user input, consider it ill-formed, so check for validity and sanitize input. Never trust user, They all try to broke your program.

              • Using std::endl sending a 'n' and then flushes the output buffer. So std::endl is more expensive in performance. So, use 'n' 'n' and then, if you want to manually flush the stream, explicitly call std::flush.


              Use your functions



              If you don't know how to use your function, I think you really have to learn to basics. Their are a lot of resources on the net. Once you get basics, dive into the C++ FAQ and the C++ Core Guideline to get some good practices.



              Also, it's Christmas soon, a good opportunity to ask for a good book about C++




              Misc



              • When it's possible, try to avoid using goto.


              • Don't return 0 from main().

              • You should also fix indentation.





              share|improve this answer






















              • Thank you so much for your feedback!
                – braisedbeefcake
                Dec 6 at 8:08










              • namespace std::string; is clearly an error, since std::string names a class, not a namespace.
                – Toby Speight
                Dec 6 at 9:55







              • 2




                return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
                – Jack Aidley
                Dec 6 at 10:27










              • @TobySpeight thx for pointing out this typo.
                – Calak
                Dec 6 at 11:48






              • 2




                @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
                – Martin York
                Dec 6 at 18:18














              up vote
              4
              down vote













              Welcome on Code Review!




              Don't use using namespace std;



              Putting using namespace std at the top of every program is a bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.



              • Although it's "safe" to use it in some place (e.g. implementation files), as long as you aren't familiar with the features of c ++, try to avoid it.

              • It led to a world of name collisions. (best case)

              • It's source of silent errors and weird bugs. (worst case)

              • If typing std:: is so tedious for you, try to import only truncated namespaces. ( e.g. individual identifiers like using std::string; or nested names like using namespace std::literals).

              • If importing nested namespaces still too awful for you, try to do it inside a restricted scope (e.g. a functions) and not in global scope.


              Ensure validity of parameters



              In your add(), substract(), multiply() and divide() function, you don't check the validity of given inputs. You could surely use assertions to ensure that the operation don't lead to integer overflow/underflow or division by zero.



              You could use assertions to check preconditions, postconditions and invariants. It make your code more explicit and you avoid possibly broke you code when modifying.




              • How to use assertions in C (for C, but still applicable here)


              • How and When to Use C's assert() Macro (same, also valid)


              • Why should I use asserts?


              Inputs / Outputs



              • When you read values from user input, consider it ill-formed, so check for validity and sanitize input. Never trust user, They all try to broke your program.

              • Using std::endl sending a 'n' and then flushes the output buffer. So std::endl is more expensive in performance. So, use 'n' 'n' and then, if you want to manually flush the stream, explicitly call std::flush.


              Use your functions



              If you don't know how to use your function, I think you really have to learn to basics. Their are a lot of resources on the net. Once you get basics, dive into the C++ FAQ and the C++ Core Guideline to get some good practices.



              Also, it's Christmas soon, a good opportunity to ask for a good book about C++




              Misc



              • When it's possible, try to avoid using goto.


              • Don't return 0 from main().

              • You should also fix indentation.





              share|improve this answer






















              • Thank you so much for your feedback!
                – braisedbeefcake
                Dec 6 at 8:08










              • namespace std::string; is clearly an error, since std::string names a class, not a namespace.
                – Toby Speight
                Dec 6 at 9:55







              • 2




                return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
                – Jack Aidley
                Dec 6 at 10:27










              • @TobySpeight thx for pointing out this typo.
                – Calak
                Dec 6 at 11:48






              • 2




                @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
                – Martin York
                Dec 6 at 18:18












              up vote
              4
              down vote










              up vote
              4
              down vote









              Welcome on Code Review!




              Don't use using namespace std;



              Putting using namespace std at the top of every program is a bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.



              • Although it's "safe" to use it in some place (e.g. implementation files), as long as you aren't familiar with the features of c ++, try to avoid it.

              • It led to a world of name collisions. (best case)

              • It's source of silent errors and weird bugs. (worst case)

              • If typing std:: is so tedious for you, try to import only truncated namespaces. ( e.g. individual identifiers like using std::string; or nested names like using namespace std::literals).

              • If importing nested namespaces still too awful for you, try to do it inside a restricted scope (e.g. a functions) and not in global scope.


              Ensure validity of parameters



              In your add(), substract(), multiply() and divide() function, you don't check the validity of given inputs. You could surely use assertions to ensure that the operation don't lead to integer overflow/underflow or division by zero.



              You could use assertions to check preconditions, postconditions and invariants. It make your code more explicit and you avoid possibly broke you code when modifying.




              • How to use assertions in C (for C, but still applicable here)


              • How and When to Use C's assert() Macro (same, also valid)


              • Why should I use asserts?


              Inputs / Outputs



              • When you read values from user input, consider it ill-formed, so check for validity and sanitize input. Never trust user, They all try to broke your program.

              • Using std::endl sending a 'n' and then flushes the output buffer. So std::endl is more expensive in performance. So, use 'n' 'n' and then, if you want to manually flush the stream, explicitly call std::flush.


              Use your functions



              If you don't know how to use your function, I think you really have to learn to basics. Their are a lot of resources on the net. Once you get basics, dive into the C++ FAQ and the C++ Core Guideline to get some good practices.



              Also, it's Christmas soon, a good opportunity to ask for a good book about C++




              Misc



              • When it's possible, try to avoid using goto.


              • Don't return 0 from main().

              • You should also fix indentation.





              share|improve this answer














              Welcome on Code Review!




              Don't use using namespace std;



              Putting using namespace std at the top of every program is a bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.



              • Although it's "safe" to use it in some place (e.g. implementation files), as long as you aren't familiar with the features of c ++, try to avoid it.

              • It led to a world of name collisions. (best case)

              • It's source of silent errors and weird bugs. (worst case)

              • If typing std:: is so tedious for you, try to import only truncated namespaces. ( e.g. individual identifiers like using std::string; or nested names like using namespace std::literals).

              • If importing nested namespaces still too awful for you, try to do it inside a restricted scope (e.g. a functions) and not in global scope.


              Ensure validity of parameters



              In your add(), substract(), multiply() and divide() function, you don't check the validity of given inputs. You could surely use assertions to ensure that the operation don't lead to integer overflow/underflow or division by zero.



              You could use assertions to check preconditions, postconditions and invariants. It make your code more explicit and you avoid possibly broke you code when modifying.




              • How to use assertions in C (for C, but still applicable here)


              • How and When to Use C's assert() Macro (same, also valid)


              • Why should I use asserts?


              Inputs / Outputs



              • When you read values from user input, consider it ill-formed, so check for validity and sanitize input. Never trust user, They all try to broke your program.

              • Using std::endl sending a 'n' and then flushes the output buffer. So std::endl is more expensive in performance. So, use 'n' 'n' and then, if you want to manually flush the stream, explicitly call std::flush.


              Use your functions



              If you don't know how to use your function, I think you really have to learn to basics. Their are a lot of resources on the net. Once you get basics, dive into the C++ FAQ and the C++ Core Guideline to get some good practices.



              Also, it's Christmas soon, a good opportunity to ask for a good book about C++




              Misc



              • When it's possible, try to avoid using goto.


              • Don't return 0 from main().

              • You should also fix indentation.






              share|improve this answer














              share|improve this answer



              share|improve this answer








              edited Dec 6 at 12:08

























              answered Dec 6 at 7:28









              Calak

              2,102318




              2,102318











              • Thank you so much for your feedback!
                – braisedbeefcake
                Dec 6 at 8:08










              • namespace std::string; is clearly an error, since std::string names a class, not a namespace.
                – Toby Speight
                Dec 6 at 9:55







              • 2




                return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
                – Jack Aidley
                Dec 6 at 10:27










              • @TobySpeight thx for pointing out this typo.
                – Calak
                Dec 6 at 11:48






              • 2




                @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
                – Martin York
                Dec 6 at 18:18
















              • Thank you so much for your feedback!
                – braisedbeefcake
                Dec 6 at 8:08










              • namespace std::string; is clearly an error, since std::string names a class, not a namespace.
                – Toby Speight
                Dec 6 at 9:55







              • 2




                return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
                – Jack Aidley
                Dec 6 at 10:27










              • @TobySpeight thx for pointing out this typo.
                – Calak
                Dec 6 at 11:48






              • 2




                @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
                – Martin York
                Dec 6 at 18:18















              Thank you so much for your feedback!
              – braisedbeefcake
              Dec 6 at 8:08




              Thank you so much for your feedback!
              – braisedbeefcake
              Dec 6 at 8:08












              namespace std::string; is clearly an error, since std::string names a class, not a namespace.
              – Toby Speight
              Dec 6 at 9:55





              namespace std::string; is clearly an error, since std::string names a class, not a namespace.
              – Toby Speight
              Dec 6 at 9:55





              2




              2




              return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
              – Jack Aidley
              Dec 6 at 10:27




              return 0; is not necessary in main but I can't see any reason to recommend against doing it. What is you reason for suggesting this?
              – Jack Aidley
              Dec 6 at 10:27












              @TobySpeight thx for pointing out this typo.
              – Calak
              Dec 6 at 11:48




              @TobySpeight thx for pointing out this typo.
              – Calak
              Dec 6 at 11:48




              2




              2




              @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
              – Martin York
              Dec 6 at 18:18




              @JackAidley By not using a return 0 is a convention that indicates that the application can never fail. Thus as a developer I don't need to look for the situations where the code will return an error. The convention says if your code can return an error (i.e. there are returns other than 0 elsewhere) then you should add the return 0 at the end. So if you see return 0 at the end you start looking for the error conditions. If you see no return 0 then you don't need to look for error situations the app is so simple it always just exits normally.
              – Martin York
              Dec 6 at 18:18










              up vote
              2
              down vote













              Default case!



              In addition to the other excellent answers, you could add a default case to your switch statement! This is the case that is called when something other than the expected input is received.



              In your example, it would end up looking something like this:



              switch (user14)

              case 1:
              Add(n1, n2);
              break;
              case 2:
              Subtract(n1, n2);
              break;
              case 3:
              Multiply(n1, n2);
              break;
              case 4:
              Divide(n1, n2);
              break;
              Default:
              cout << "Invalid operationn";
              break;



              In the above code, a 1, 2, 3, or 4 will perform the expected corresponding action, as in your original code. But in your original code, any other numerical input will just exit the switch statement and move on to the next thing, without telling the user that no action was taken. In the example above, it will instead tell the user why no action is being taken.



              It's a useful way to let the user know that their input was invalid, but it can also be used to automatically take an action if none of the cases are matched. If that is confusing, you can think of default like the final "else" at the end of a series of "if/else if" statements.



              On the other hand, if there's no need for user feedback and there's no "default" action that you want to happen normally, then there's no need to make a default case.



              Another change I might suggest is for user14, I would personally use a char instead of an int. This will stop the program from crashing if the user inputs something other than an integer. But then, if you know that the input will only ever be an integer, then there's really no need to take that precaution!



              Happy coding!






              share|improve this answer
























                up vote
                2
                down vote













                Default case!



                In addition to the other excellent answers, you could add a default case to your switch statement! This is the case that is called when something other than the expected input is received.



                In your example, it would end up looking something like this:



                switch (user14)

                case 1:
                Add(n1, n2);
                break;
                case 2:
                Subtract(n1, n2);
                break;
                case 3:
                Multiply(n1, n2);
                break;
                case 4:
                Divide(n1, n2);
                break;
                Default:
                cout << "Invalid operationn";
                break;



                In the above code, a 1, 2, 3, or 4 will perform the expected corresponding action, as in your original code. But in your original code, any other numerical input will just exit the switch statement and move on to the next thing, without telling the user that no action was taken. In the example above, it will instead tell the user why no action is being taken.



                It's a useful way to let the user know that their input was invalid, but it can also be used to automatically take an action if none of the cases are matched. If that is confusing, you can think of default like the final "else" at the end of a series of "if/else if" statements.



                On the other hand, if there's no need for user feedback and there's no "default" action that you want to happen normally, then there's no need to make a default case.



                Another change I might suggest is for user14, I would personally use a char instead of an int. This will stop the program from crashing if the user inputs something other than an integer. But then, if you know that the input will only ever be an integer, then there's really no need to take that precaution!



                Happy coding!






                share|improve this answer






















                  up vote
                  2
                  down vote










                  up vote
                  2
                  down vote









                  Default case!



                  In addition to the other excellent answers, you could add a default case to your switch statement! This is the case that is called when something other than the expected input is received.



                  In your example, it would end up looking something like this:



                  switch (user14)

                  case 1:
                  Add(n1, n2);
                  break;
                  case 2:
                  Subtract(n1, n2);
                  break;
                  case 3:
                  Multiply(n1, n2);
                  break;
                  case 4:
                  Divide(n1, n2);
                  break;
                  Default:
                  cout << "Invalid operationn";
                  break;



                  In the above code, a 1, 2, 3, or 4 will perform the expected corresponding action, as in your original code. But in your original code, any other numerical input will just exit the switch statement and move on to the next thing, without telling the user that no action was taken. In the example above, it will instead tell the user why no action is being taken.



                  It's a useful way to let the user know that their input was invalid, but it can also be used to automatically take an action if none of the cases are matched. If that is confusing, you can think of default like the final "else" at the end of a series of "if/else if" statements.



                  On the other hand, if there's no need for user feedback and there's no "default" action that you want to happen normally, then there's no need to make a default case.



                  Another change I might suggest is for user14, I would personally use a char instead of an int. This will stop the program from crashing if the user inputs something other than an integer. But then, if you know that the input will only ever be an integer, then there's really no need to take that precaution!



                  Happy coding!






                  share|improve this answer












                  Default case!



                  In addition to the other excellent answers, you could add a default case to your switch statement! This is the case that is called when something other than the expected input is received.



                  In your example, it would end up looking something like this:



                  switch (user14)

                  case 1:
                  Add(n1, n2);
                  break;
                  case 2:
                  Subtract(n1, n2);
                  break;
                  case 3:
                  Multiply(n1, n2);
                  break;
                  case 4:
                  Divide(n1, n2);
                  break;
                  Default:
                  cout << "Invalid operationn";
                  break;



                  In the above code, a 1, 2, 3, or 4 will perform the expected corresponding action, as in your original code. But in your original code, any other numerical input will just exit the switch statement and move on to the next thing, without telling the user that no action was taken. In the example above, it will instead tell the user why no action is being taken.



                  It's a useful way to let the user know that their input was invalid, but it can also be used to automatically take an action if none of the cases are matched. If that is confusing, you can think of default like the final "else" at the end of a series of "if/else if" statements.



                  On the other hand, if there's no need for user feedback and there's no "default" action that you want to happen normally, then there's no need to make a default case.



                  Another change I might suggest is for user14, I would personally use a char instead of an int. This will stop the program from crashing if the user inputs something other than an integer. But then, if you know that the input will only ever be an integer, then there's really no need to take that precaution!



                  Happy coding!







                  share|improve this answer












                  share|improve this answer



                  share|improve this answer










                  answered Dec 6 at 21:03









                  MrSpudtastic

                  1211




                  1211



























                      draft saved

                      draft discarded
















































                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid


                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.

                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.





                      Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                      Please pay close attention to the following guidance:


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid


                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.

                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function ()
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209130%2fbeginner-c-calculator%23new-answer', 'question_page');

                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown






                      Popular posts from this blog

                      How to check contact read email or not when send email to Individual?

                      Displaying single band from multi-band raster using QGIS

                      How many registers does an x86_64 CPU actually have?