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

working version #5

Open
wants to merge 1 commit 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: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ project(dummy_cmake_project)
set(CMAKE_CXX_STANDARD 14)
list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake)

set(SOURCES project.cc)
set(SOURCES project.cc Parser.cpp Token.cpp Stack.cpp Calculator.cpp executor.cpp)
add_library(ProjectLib ${SOURCES})

add_executable(project main.cc)
add_executable(project main.cc Parser.cpp Token.cpp Stack.cpp Calculator.cpp executor.cpp)
target_link_libraries(project ProjectLib)

enable_testing()
Expand Down
49 changes: 49 additions & 0 deletions Calculator.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include "Calculator.h"
#include <iostream>

Calculator::Calculator(Stack &stk) : stk_(stk)
{}

double Calculator::exe() {

std::vector<long double> numbers;
double rec = 0;
for(;;) {
Copy link
Owner

Choose a reason for hiding this comment

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

this for definitely should be splitted

int rc;
Token t = stk_.pop(rc);
if(t.is_number()) {
numbers.push_back(t.getNumber());
} else { //operation

long double result = numbers[0];
for(int i = 1; i < numbers.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.

Start splitting from internal scopes.


switch (t.getOperand()) {
case TOper::ADD:
result += numbers[i];
break;
case TOper::SUB:
result -= numbers[i];
break;
case TOper::MUL:
result *= numbers[i];
break;
case TOper::DIV:
result /= numbers[i];
break;
default:
std::cout << "exe error" << std::endl;
break;
}
}
numbers.clear();
Token nt(true, TOper::NONE, result);
stk_.push(nt);
std::cout << result << std::endl;
rec = result;
}

if(!rc) break;
Copy link
Owner

Choose a reason for hiding this comment

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

rename rc :-)

}
return rec;
}
16 changes: 16 additions & 0 deletions Calculator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#ifndef CALCULATOR_H
#define CALCULATOR_H

#include "Stack.h"

class Calculator
{
public:
Calculator(Stack &stk);
double exe();///TODO rename execute

private:
Stack stk_;
};

#endif // CALCULATOR_H
49 changes: 49 additions & 0 deletions Parser.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include "Parser.h"

#include <iostream> //just for test
#include <string>

Parser::Parser(const std::vector <std::string>& line)
: rawLines(line)
, currentPosition(0)
{}

void Parser::parse()
{
for(size_t i = 0; i < rawLines.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.

cac be used range based?

currentPosition = i+1;
//check for operator
///TODO: check Factory applicance
if(rawLines[i] == std::string("+")) {
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like good place for factory

Token t(TOper::ADD);
tokens.push_back(t);
} else if (rawLines[i] == std::string("-")) {
Token t(TOper::SUB);
tokens.push_back(t);
} else if (rawLines[i] == std::string("*")) {
Token t(TOper::MUL);
tokens.push_back(t);
} else if (rawLines[i] == std::string("/")) {
Token t(TOper::DIV);
tokens.push_back(t);
} else { //check for number
Copy link
Owner

Choose a reason for hiding this comment

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

Long ifs with else inside for smells

long double dd;
// try {
dd = std::stod (rawLines[i]);
//try to throw position also
// } catch (...) {
Copy link
Owner

Choose a reason for hiding this comment

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

Try to avoid commented code.

// std::cerr << "wrong data on " << i+1
// << " position" << std::endl;
// return;
// }

Token t(dd);
tokens.push_back(t);
}
}
}

const std::vector<Token> &Parser::getTokens()
{
return tokens;
}
20 changes: 20 additions & 0 deletions Parser.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#ifndef PARSER_H
#define PARSER_H

#include "Token.h"
#include <vector>

class Parser
{
public:
Parser(const std::vector <std::string>& line);
void parse();
const std::vector<Token>& getTokens();
Copy link
Owner

Choose a reason for hiding this comment

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

Methods are too highly coupled. If you can't use getTokens if parse was unsuccessful and so on.

size_t getPositionOfErrorInRawLine() const;
Copy link
Owner

Choose a reason for hiding this comment

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

Not clear result if there is no error

private:
std::vector<Token> tokens;
std::vector <std::string> rawLines;
Copy link
Owner

Choose a reason for hiding this comment

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

Think about life time of entities. If you don;t have a reason keeping entity any more, there is no reason to keep it in class members.

size_t currentPosition;
};

#endif // PARSER_H
45 changes: 45 additions & 0 deletions Stack.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#include "Stack.h"
#include <iostream>

Stack::Stack(const std::vector<Token>& t) : size(0) /*: size(t.size()) */ //init vector
{
//for(int i = size-1; i >= 0; --i) {
for(int i = t.size()-1; i >= 0; --i) {
Copy link
Owner

Choose a reason for hiding this comment

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

range based or iterator based approach prefered

push(t[i]);
}
}

Stack::~Stack()
{
size = 0;
stk.clear();
}

int Stack::getSize() const
{
return size;
}

void Stack::push(const Token& elem)
Copy link
Owner

Choose a reason for hiding this comment

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

Think about error handling. cout is bad error handling

{
if ( stk.size() <= size ) { // here size must be added after compate
stk.push_back(elem);
++size;
} else
std::cout << "stack is full" << std::endl;
}

Token Stack::pop(int &rc)
{
rc = 1;
if ( stk.empty() ) {
std::cout << "stack is empty" << std::endl;
//return -EINVAL;
rc = 0;
Copy link
Owner

Choose a reason for hiding this comment

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

Yu are mixing business values and error codes. If stack is full you will return 0.

} else {
Copy link
Owner

Choose a reason for hiding this comment

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

This else can be removed with early return

Token temp = stk.back();
stk.pop_back();
--size;
return temp;
}
}
22 changes: 22 additions & 0 deletions Stack.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#ifndef STACK_H
#define STACK_H

#include <stdint.h>
#include <vector>
#include "Token.h"

///TODO:namespace
class Stack {
int size;
Copy link
Owner

Choose a reason for hiding this comment

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

Do you need separate size var? can youdelegate it to vector?

std::vector<Token> stk;
public:
///TODO: stack is stack. do adapter for stack with Token
Stack(const std::vector<Token>& t);
~Stack();
void push(const Token& elem);
Token pop(int &rc); ///TODO: think about another one interface
int getSize() const;
};


#endif // STACK_H
40 changes: 40 additions & 0 deletions Token.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#include "Token.h"

Token::Token(const bool is_number, const TOper::Operand opr, const long double num)
: is_num(is_number)
{
if(is_num) {
d = num;
this->opr = TOper::NONE;
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like NONE mens NUMBER, isn't it?

} else {
this->opr = opr;
Copy link
Owner

Choose a reason for hiding this comment

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

Early return prefered

d = 0; //default
}
}

Token::Token(const long double num)
: is_num(true)
, d(num)
Copy link
Owner

Choose a reason for hiding this comment

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

What is d? Bad Naming

, opr(TOper::NONE)
Copy link
Owner

Choose a reason for hiding this comment

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

opr is also bad naming

{}

Token::Token(const TOper::Operand opr)
: is_num(false)
, d(0)
, opr(opr)
{}

TOper::Operand Token::getOperand()
{
return opr;
}

long double Token::getNumber()
{
return d;
}

bool Token::is_number()
{
return is_num;
}
36 changes: 36 additions & 0 deletions Token.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#ifndef TOKEN_H
#define TOKEN_H

#include <string>

namespace TOper {

///TODO: rename
///TODO: enum class
enum Operand {
ADD,
SUB,
DIV,
MUL,
NONE
Copy link
Owner

Choose a reason for hiding this comment

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

I would say that NONE is number in your class, and Operand looks like TokenType

};

}

class Token
{
public:
///TODO: split to different constructors
Token(const bool is_number, const TOper::Operand opr, const long double num);
Token(const long double num);
Token(const TOper::Operand opr);
Copy link
Owner

Choose a reason for hiding this comment

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

3 constructs with partial initialization confuses.

bool is_number();
Copy link
Owner

Choose a reason for hiding this comment

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

sounds like const

TOper::Operand getOperand();
long double getNumber();
private:
long double d;
TOper::Operand opr;
bool is_num;
};

#endif // TOKEN_H
16 changes: 16 additions & 0 deletions executor.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#include "executor.h"

#include "Parser.h"
#include "Stack.h"
#include "Calculator.h"
#include "executor.h"

double execute(const std::vector <std::string>& src)
{
Parser p(src);
p.parse();
Stack stk(p.getTokens());
//std::cout << "test" << std::endl;
Calculator calc(stk);
return calc.exe();
}
9 changes: 9 additions & 0 deletions executor.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#ifndef EXECUTOR_H
#define EXECUTOR_H

#include <vector>
#include <string>
///TODO: namespace wrrap
double execute(const std::vector <std::string>& src);

#endif // EXECUTOR_H
63 changes: 61 additions & 2 deletions main.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,64 @@
#include "project.h"

int main() {
return 0;
#include "executor.h"

#include <iostream>
#include <vector>
#include <string>
using namespace std; // test

/*
* The calculator input is double numbers, operators + - * /
* The maximum input line is 255
*
* */

static int show_usage()
{
std::cerr << "Usage: <option(s)> || <valid arguments>"
<< "Options:\n"
<< "\t-h,--help\t\tShow this help message\n"
<< "Valid arguments:\n"
<< "\tSpecify target string after program name\n"
<< "\tNumbers with point and operations + - / * with space separation"
<< std::endl;
return -1;
}

int main(int argc, char** argv) {

try {
if (argc < 2 /*|| argc >= 12*/) {
show_usage();
Copy link
Owner

Choose a reason for hiding this comment

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

Can you throw here?

return 1;
}

std::vector <std::string> sources;
for (int i = 1; i < argc; ++i) {
Copy link
Owner

Choose a reason for hiding this comment

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

I would put htis for in separate function

std::string arg = argv[i];
if ((arg == "-h") || (arg == "--help")) {
show_usage();
return 0;
} else {
sources.push_back(arg);
}
}

execute(sources);
} catch (const std::invalid_argument& e) {
std::cout << "std::invalid_argument" << std::endl;
show_usage();
} catch (const std::out_of_range& e) {
std::cout << "std::out_of_range" << std::endl;
show_usage();
} catch (...) {
/*
If no conversion could be performed, an invalid_argument exception is thrown.
If the value read is out of the range of representable values by a double (in some library implementations, this includes underflows), an out_of_range exception is thrown.
*/

show_usage();
}

return 0;
}
Loading