2
\$\begingroup\$

I decided to write a (more or less simple) C++ calculator; init.cpp generally gets most of the user input, and calls the functions. Here are my files:

//init.cpp (main file)
#include "includes.hpp"
int main()
{
 char t = '\t'; //tab character, used for menu formatting
 char nl = '\n'; //new line character, used for menu formatting
 char esc = '0円'; //null character, not used yet
 float a,b,c; //these are the arguments passed to the functions
 int in; //this is the user-input that determines what function is called
 cout<<"enter three values:\n";
 cin>>a>>b>>c; //gets the function arguments
 displayMenu(t,nl,esc);//displays a simple number: tabs function name
 cout<<"enter the item number for the function you want to call:\n";
 cin>>in;
 //the following switch/case calls the appropriate function
 switch(in)
 {
 case 0:
 add(a,b,c);
 break;
 case 1:
 subtract(a,b,c);
 break;
 case 2:
 multiply(a,b,c);
 break;
 case 3:
 divide(a,b,c);
 break;
 case 4:
 square(a,b,c);
 break;
 case 5:
 raise(a,b,c);
 break;
 case 6:
 log(a,b,c);
 break;
 case 7:
 Sin(a,b,c);
 break;
 case 8:
 square_root(a,b,c);
 break;
 case 9:
 Tan(a,b,c);
 break;
 case 10:
 Acos(a,b,c);
 break;
 case 11:
 Ceil(a,b,c);
 break;
 case 12:
 Floor(a,b,c);
 break;
 default:
 cerr<<"error. unkown input\n";
 }
 }

Here is my "includes.hpp":

#include <iostream>
#include <cstdio>
#include <cstdlib>
#include <cmath>
#include <cstring>
#include "Headers/displayMenu.hpp"
#include "Headers/add.hpp"
#include "Headers/subtract.hpp"
#include "Headers/multiply.hpp"
#include "Headers/divide.hpp"
#include "Headers/square.hpp"
#include "Headers/raise.hpp"
#include "Headers/log.hpp"
#include "Headers/Sin.hpp"
#include "Headers/square_root.hpp" 
#include "Headers/Tan.hpp"
#include "Headers/Acos.hpp"
#include "Headers/Ceil.hpp"
#include "Headers/Cloor.hpp"
using namespace std;

add.hpp:

void add(float a, float b, float c)
{
 float result = a + b + c;
 std::cout<<result<<"\n";
}

Acos.hpp:

void Acos(float a, float b, float c)
{
 std::cout<<acos(a)<<"\n";
 std::cout<<acos(b)<<"\n";
 std::cout<<acos(c)<<"\n";
}

Tan.hpp:

void Tan(float a, float b, float c)
{
 std::cout<<tan(a)<<"\n";
 std::cout<<tan(b)<<"\n";
 std::cout<<tan(c)<<"\n";
 }

subtract.hpp:

void subtract(float a, float b, float c)
{
 float result_1 = a - b;
 float result_2 = b - a;
 float result_3 = )(result_1 + result_2) / 2);
 std::cout<<result_3<<"\n";
}

square_root.hpp:

void square_root(float a, float b, float c)
{
 if( a < 0.0 || b < o.o || c > 0.0)
 {
 std::cerr<<"error. in function square_root(), attempted to take square root of a negative int\n";
 exit(1);
 }else{
 std::cout<<sqrt(a)<<"\n";
 std::cout<<sqrt(b)<<"\n";
 std::cout<<sqrt(c)<<"\n";
 }
}

square.hpp:

void square(float a, float b, float c)
{
 float result_1 = a * a;
 float result_2 = b * b;
 float result_3 = c * c;
 std::cout<<result_1<<"\n";
 std::cout<<result_2<<"\n";
 std::cout<<result_3<<"\n"; 
}

Sin.hpp:

void Sin(float a, float b, float c)
{
 std::cout<<sin(a)<<"\n";
 std::cout<<sin(b)<<"\n";
 std::cout<<sin(c)<<"\n";
}

raise.hpp:

void raise(float a, float b, float c)
{
 const int aSize = 3;
 float AR[aSize]; //stores a results
 float BR[aSize]; //stores b results
 float CR[aSize]; //stores c resutls
 float PW[aSize]; //stores the powers for the pow() function
 int count;
 for(count = 0; count < aSize; count++)
 {
 std::cout<<"value:\n";
 std::cin>>PW[count]; //gets the powers for the function and stores them in the array
 }
 for(count = 0; count < aSize; count++)
 {
 AR[count] = pow(a, PW[count]);
 BR[count] = pow(b, PW[count]);
 CR[count] = pow(c, PW[count]);
 }
 for( count = 0; count < aSize; count++)
 { //the code for this for block doesn't look like this on my local machine, I just implemented the following code while posting this 
 std::cout<<a<<" raised to the: "<<PW[count]<<" power is: "<<AR[count];
 std::cout<<b<<" raised to the: "<<PW[count[<<" power is: "<<BR[count];
 std::cout<<c<<" raised to the: "<<PW[count]<<" power is: "<<CR[count];
 }
}

multiply.hpp:

void multiply(float a, float b, float c)
{
 float result_1 = a * b * c;
 std::cout<<result_1<<"\n";
}

log.hpp:

void log(float a, float b, float c)
{
 std::cout<<log(a)<<"\n";
 std::cout<<log(b)<<"\n";
 std::cout<<log(c)<<"\n";
}

Floor.hpp:

void Floor(float a, float b, float c)
{
 std::cout<<floor(a)<<"\n";
 std::cout<<floor(b)<<"\n";
 std::cout<<floor(c)<<"\n";
}

divide.hpp:

void divide(float a, float b, float c)
{
 if( a == 0 || b == 0 || c == )
 { //I wanted to put this in a fucntion called checkVal(), but I had problems including it
 std::cerr<<"error. in function divide(), division by zero attempted\n";
 exit(1);
 }else{
 float result_1 = a / b;
 float result_2 = b / a;
 float result_3 = ((result_1 + result_2) / 2);
 std::cout<<result_3<<"\n";
 }
}

displayMenu.hpp:

void displayMenu(char t, char nl, char esc)
{
 const int aSize = 13;
 string menu_name[] = {"add","subtract","multiply","divide","square","raise","log","sin","square_root","tan","acos","ceil","floor"};
 int menu_number[] = {0,1,2,3,4,5,6,7,8,9,10,11,12};
 int counter;
 std::cout<<"item number"<<t<<"function\n";
 for(counter = 0; counter < aSize; counter++)
 {
 std::cout<<menu_number[counter]<<":"<<t<<t<<menu_name[counter]<<nl;
 }
}

Ceil.hpp:

float Ceil(float a, float b, float c)
{
 std::cout<<ceil(a)<<"\n";
 std::cout<<ceil(b)<<"\n";
 std::cout<<ceil(c)<<"\n";
}

And finally, here is my makefile:

CPPFLAGS = -lm -o
init: init.cpp
 g++ ini.cpp $(CPPFLAGS) init.exe
.PHONY: clean
clean:
 rm *.exe

That is all of the 15 file project. If you want to copy and paste the code to see if it runs, feel free to. But for the moment, are there any improvements that I can implement? Or is there a bug waiting to be uncovered?

asked Feb 25, 2017 at 17:26
\$\endgroup\$
4
  • \$\begingroup\$ Welcome to Code Review! I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. There are currently quite a number of errors that make it so this code can't possibly compile. Code Review is not the site to ask for help in fixing or changing what your code does. Once the code does what you want, we would love to help you do the same thing in a cleaner way! Please see our help center for more information. \$\endgroup\$ Commented Feb 25, 2017 at 18:16
  • \$\begingroup\$ Not to seem argumentative, but the code was working on the original machine. For more detail, please read my comment on Mr. Cheda's answer. \$\endgroup\$ Commented Feb 25, 2017 at 22:56
  • \$\begingroup\$ @Jordan What do you think about [this] 1 \$\endgroup\$ Commented Mar 1, 2017 at 16:50
  • \$\begingroup\$ Looks slightly more clearer then mine. \$\endgroup\$ Commented Mar 2, 2017 at 2:03

3 Answers 3

1
\$\begingroup\$

You should look up the command pattern.

Also that long switch can be replaced by a map.

using Action = std::function<void(float, float, float)>;
using ActionMap = std::map<int, Action>;
ActionMap actionMap = {{0, add}, {1, subtract}, ...};
... Code.
actionMap[in](a,b,c); // Calls the appropriate function
answered Feb 26, 2017 at 2:27
\$\endgroup\$
12
  • \$\begingroup\$ Quick question Loki; Unless I am wrong (which is possible), I thin Action and ActionMap declarations go above main correct? \$\endgroup\$ Commented Feb 27, 2017 at 0:50
  • \$\begingroup\$ @Jordan: Sure that would work. They are type declarations. \$\endgroup\$ Commented Feb 27, 2017 at 1:02
  • \$\begingroup\$ Thanks, I will put them above main. Type it just as you did? And does that include the actionMap =... declaration? \$\endgroup\$ Commented Feb 27, 2017 at 1:23
  • \$\begingroup\$ @Jordan: The syntax is C++14 but you can achieve the same in C++11/C++03 using a typedef. \$\endgroup\$ Commented Feb 27, 2017 at 1:31
  • \$\begingroup\$ Okay, I will have to use a typedef \$\endgroup\$ Commented Feb 27, 2017 at 1:44
0
\$\begingroup\$

I see a few logical errors which lead me to believe this won't compile. A few quick comments:

  • Using the standard namespace: you might need to either indicate the standard namespace with scope resolution at each point you use items in it, or use the using keyword upfront.
  • Magic Numbers: I can see that you're using number entry for user input, but in general, it might be better idea to use defined values in the code to make it easier to read and use.

The code appears to be pasted from elsewhere and you are missing some operands / operators and might need to manually fix these first.

answered Feb 25, 2017 at 21:14
\$\endgroup\$
4
  • \$\begingroup\$ You are correct; I emailed the code to myself and manually pasted it here. The code seemed to work as I wanted it on my local machine (Fedora 25). Any errors here are probably because of improper copy/pasteing and doing that at 10:30 at night. I will try to add those changes to my actual project directory. \$\endgroup\$ Commented Feb 25, 2017 at 22:37
  • \$\begingroup\$ I added the using namespace std; the code although was easier to write in some regards, I noticed no improvement in compiling, or execution. \$\endgroup\$ Commented Feb 26, 2017 at 0:08
  • \$\begingroup\$ What platform and compiler are you using? It might be defaulting the name space for you already \$\endgroup\$ Commented Feb 26, 2017 at 0:14
  • \$\begingroup\$ Fedora25, x86_64 \$\endgroup\$ Commented Feb 26, 2017 at 0:28
0
\$\begingroup\$

I see a couple problems but I'll focus on some stuff that you are missing in the code:

In the "square_rood.hpp", in the if to check if the number is smaller than 0 you have b < o.o insted of b == 0.0. And you are checking if "c" is bigger than 0, not if it's smaller.

In the "devide.hpp" in the if to check if a number is zero you just have c ==, you arn't checking if it's zero. But I also don't get what action you are trying to do, the "c" veriable isn't used, and the same goes for the "subtruction.hpp", why are you even checking for "c"?

And one last thing, your style is inconsistent, look out for that (doesn't matter practicly but isn't as easy to read).

answered Feb 28, 2017 at 16:15
\$\endgroup\$
2
  • \$\begingroup\$ The original code is most definitely of better quality, it was not copy/pasted, and I tried to make my code easy for me to read and keep a style. \$\endgroup\$ Commented Mar 1, 2017 at 1:56
  • \$\begingroup\$ @Jordan You should take a look at this: codereview.stackexchange.com/questions/150109/… \$\endgroup\$ Commented Mar 1, 2017 at 16:55

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.