Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calc #9

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake)
set(SOURCES project.cc)
add_library(ProjectLib ${SOURCES})

set(SOURCES polishNotationCalc.cpp)
add_library(polishCalcLib ${SOURCES})

add_executable(project main.cc run.cc)
target_link_libraries(project ProjectLib)
target_link_libraries(project ProjectLib polishCalcLib)

enable_testing()
add_subdirectory(test)
92 changes: 92 additions & 0 deletions polishNotationCalc.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
#include "polishNotationCalc.hpp"
namespace dev
{
PolishCalc::PolishCalc()
{
insertFunctionInsideMap();
}

Copy link
Contributor

@LuxoftAKutsan LuxoftAKutsan Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 Step : Create Token stack and validate it

token_stack& =  stack<Tocken>

Token {
  // Process one tocken in the stack 
    void process(token_stack& ) 
    double value(const token_stack& )  << // Follow contant 
}

NumericToken  : Token {
  void  process(token_stack& ) -> do nothing
  double  value(token_stack& ) -> return value_
  double value_
} 

OperatorTocken : Token {
  pair PopTockens 
}

PlusTocken  : OperatorTocken {
  void  process(token_stack& ) { 
     pop token from stack -> get value -> double res1
     pop token2 from stack -> get value -> double res2
     res1 + res2 -> Plus result 
     Push plus result to token vector  as Numeric 
  }
  
  double  value(const token_stack&) {
    // Option 1 Contract thet process() should be evaluated before value ()
     return top of the token_stack& value() 
  
    // Option 2 Base that there process() could not be called
         get token from stack -> get value -> double res1
         get token2 from stack on the second position -> get value -> double res2 
         res1 + res2 -> Plus result 
         return Plus result  as double 
  } 
} 

3 step Go throw all tokens and call process to them

bool PolishCalc::isNumeric(const char num)
{
double temp = num - '0';
if (temp >= 0 && temp <= 9)
return true;

return false;
}
bool PolishCalc::isOperator(const char num)
{
if (num == '+' || num == '-' || num == '*' || num == '/')
return true;
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for (from 0 to N) { 
 Operation 1
 Operation 2
 Operation 3
}
for (from 0 to N) { 
 Opertation 1
}
for (from 0 to N) { 
 Opertation 2
}
for (from 0 to N) { 
 Opertation 3
}

Both solutions have the same performance


double PolishCalc::calc( const std::string &str)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 blocks :

1. Basic Validation // Check that the is not bad symbols and so on
2. Parsing // Use not strings, but OOP types or lambdas for futher working 
2.1 Validation : count of operator matches count of operands 
3. Calculation // Evaluete expression 

2 stacks :
Validation block fill up 2 stacks :

  1. Operator stack
  2. Operand stack

Calculation should work with Operator/Operand or lambda types but not string.

{
if (str.size() == 0)
throw Exception{"Nothing to work on as Input is empty"};
for (int i = 0; i < str.size(); i++)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to use iterator or range based loop?

{
if (str[i] == ' ')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid missing brackets even in one line if/else/while ...

continue;
if (isNumeric(str[i]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to modify solution to work with negative numbers

{
pushOperandInStack(str, i);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to work with some kind of tokens but not charters.
It is not obvious that pushOperandInStack will modify i variable. If it is possible. Try to avoid it


}
if (isOperator(str[i]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create separate loop to fill operator stack, operand stack an calculation based on 2 stacks

{
auto operandVal = getOperandFromStack();
double res = 0;
res = mOperatorMap[str[i]](operandVal.first, operandVal.second);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move calculation to the third loop

mCalculatorStack.push(res);
}
}

if (mCalculatorStack.size() > 1)
throw Exception{"Not a valid string"};

return mCalculatorStack.top();
}

void PolishCalc::pushOperandInStack(const std::string &str, int& curr_pos)
{
double operand = 0;
std::string temp = "";
while (curr_pos < str.size() && !(str[curr_pos] == ' '))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of input like 2 4+ , solution behaviour is undefined.
Try to handle this error.
Best option is to create prevalidation function and contract for pushOperandInStack.

{
temp = temp + str[curr_pos];
curr_pos++;
}
operand = atof(temp.c_str());
mCalculatorStack.push(operand);
curr_pos--;

}
std::pair<double, double> PolishCalc::getOperandFromStack()
{
double secOperand = 0;
double firstOperand = 0;
if (mCalculatorStack.size() < 2)
{
throw Exception{"No operand to work on"};
}
secOperand = mCalculatorStack.top();
mCalculatorStack.pop();
firstOperand = mCalculatorStack.top();
mCalculatorStack.pop();
return std::make_pair(firstOperand, secOperand);
}
void PolishCalc::insertFunctionInsideMap()
{
mOperatorMap.insert(std::make_pair('+', [](const double a, const double b) { return a + b; }));
mOperatorMap.insert(std::make_pair('-', [](const double a, const double b) { return a - b; }));
mOperatorMap.insert(std::make_pair('*', [](const double a, const double b) { return a * b; }));
mOperatorMap.insert(std::make_pair('/', [](const double a, const double b) {
if(b==0)
Copy link
Contributor

@LuxoftAKutsan LuxoftAKutsan Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if(0=b)

{
throw Exception{"divide by zero"};
}
return a / b; }));
}
} // namespace dev
49 changes: 49 additions & 0 deletions polishNotationCalc.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#pragma once
#include <iostream>
#include <stack>
#include <utility>
#include <functional>
#include <string>
#include <iterator>
#include <map>

namespace dev
{
/*!
* \brief Exception class for Calculator.
*/
class Exception final : public std::exception
{
public:
template <typename T>
Exception(T &&msg)
: mMsg(std::forward<T>(msg))
{
}

const char *what() const noexcept override
{
return mMsg.c_str();
}

private:
const std::string mMsg;
};
using functionCall = std::function<double(double, double)>;
class PolishCalc
{
public:
double calc(const std::string &);
PolishCalc();

private:
bool isNumeric(const char num);
bool isOperator(const char num);
void insertFunctionInsideMap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to use constexpr for this.

void pushOperandInStack(const std::string &str, int& curr_pos);
std::pair<double, double> getOperandFromStack();
std::stack<double> mCalculatorStack;
std::map<const char, functionCall> mOperatorMap;
};

} // namespace dev
1 change: 1 addition & 0 deletions project.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ namespace dev {
int Project::run() {
return 0;
}

} // namespace dev
17 changes: 10 additions & 7 deletions project.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
#pragma once
#include "iproject.h"
#include "string"

namespace dev {
namespace dev
{

class Project : public IProject {
// IProject interface
public:
int run();
};
} // namespace dev
class Project : public IProject
{
// IProject interface
public:
int run();
};
} // namespace dev
3 changes: 3 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ target_link_libraries(ProjectTest PUBLIC ProjectLib)

add_executable (RunTest ${CMAKE_SOURCE_DIR}/run.cc run_tests.cc)
add_gtest(RunTest)

add_executable (CalculatorTest ${CMAKE_SOURCE_DIR}/polishNotationCalc.cpp calculator_test.cpp)
add_gtest(CalculatorTest)
53 changes: 53 additions & 0 deletions test/calculator_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#include "project.h"
#include "polishNotationCalc.hpp"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
namespace dev
{
namespace testing
{

class CalculatorTest : public ::testing::Test
{
public:
void SetUp() override {}
void TearDown() override {}
dev::PolishCalc project_;
};
TEST_F(CalculatorTest, BasicOPTestWithInt)
{
EXPECT_EQ(7, project_.calc("5 2 +"));
}
TEST_F(CalculatorTest, BasicOPTestWithDouble)
{
EXPECT_EQ(7.8, project_.calc("5.5 2.3 +"));
}
TEST_F(CalculatorTest, MissingOperand)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negative and positive test cases.

{
EXPECT_ANY_THROW(project_.calc("5 +"));
}
TEST_F(CalculatorTest, MissingOperator)
{
EXPECT_ANY_THROW(project_.calc("5 2"));
}
TEST_F(CalculatorTest, EmptyString)
{
EXPECT_ANY_THROW(project_.calc(""));
}
TEST_F(CalculatorTest, DivideByZero)
{
EXPECT_ANY_THROW(project_.calc("7 0 /"));

}
TEST_F(CalculatorTest, PerformComplexOpeartions)
{
EXPECT_EQ(405, project_.calc("50 31 + 10 5 - * "));
}

TEST_F(CalculatorTest, MixingDoubleAndInt)
{
auto res = (int)project_.calc("12.5 3 / ");
EXPECT_EQ(4, res);
}
}
}
31 changes: 18 additions & 13 deletions test/project_test.cc
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
#include "project.h"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
namespace dev {
namespace testing {
namespace dev
{
namespace testing
{

class ProjectTest : public ::testing::Test {
public:
void SetUp() override {}
void TearDown() override {}
dev::Project project_;
};
class ProjectTest : public ::testing::Test
{
public:
void SetUp() override {}
void TearDown() override {}
dev::Project project_;
};

TEST_F(ProjectTest, Run) {
ASSERT_EQ(0, project_.run());
}
TEST_F(ProjectTest, Run)
{
ASSERT_EQ(0, project_.run());
}

} // namespace testing
} // namespace dev
} // namespace testing
// namespace testing
} // namespace dev