-
Notifications
You must be signed in to change notification settings - Fork 9
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
base: master
Are you sure you want to change the base?
Conversation
c6716c2
to
456cebe
Compare
|
||
std::vector<long double> numbers; | ||
double rec = 0; | ||
for(;;) { |
There was a problem hiding this comment.
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
} else { //operation | ||
|
||
long double result = numbers[0]; | ||
for(int i = 1; i < numbers.size(); ++i) { |
There was a problem hiding this comment.
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.
rec = result; | ||
} | ||
|
||
if(!rc) break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename rc :-)
|
||
void Parser::parse() | ||
{ | ||
for(size_t i = 0; i < rawLines.size(); ++i) { |
There was a problem hiding this comment.
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("+")) { |
There was a problem hiding this comment.
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
///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); |
There was a problem hiding this comment.
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.
Token(const bool is_number, const TOper::Operand opr, const long double num); | ||
Token(const long double num); | ||
Token(const TOper::Operand opr); | ||
bool is_number(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds like const
|
||
try { | ||
if (argc < 2 /*|| argc >= 12*/) { | ||
show_usage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you throw here?
} | ||
|
||
std::vector <std::string> sources; | ||
for (int i = 1; i < argc; ++i) { |
There was a problem hiding this comment.
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
ASSERT_EQ(4, stk.getSize()); | ||
|
||
t = stk.pop(rc); | ||
ASSERT_EQ(1, rc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long test. Probably can be splitted
No description provided.