Make the compile work on a Mac #2
Conversation
src/quantity.cpp
Outdated
| std::string regexpstr = "([0-9]+)(\\.[0-9]*)? *([^ \t]*)"; | ||
| std::regex reg(regexpstr); | ||
| std::smatch match; | ||
| std::regex_search(str, match, reg); | ||
|
|
||
| if ( !match.empty() ) { | ||
| this->value = std::stod(match[1].str() + match[2].str()); | ||
| this->unit = match[3]; | ||
| } else { | ||
| this->value = 0.0; | ||
| this->unit = ""; | ||
| } |
There was a problem hiding this comment.
Minor nit: this block uses spaces for indentation, while the rest of the code uses tabs. I'd suggest we use tabs here too for consistency
src/quantity.cpp
Outdated
| std::smatch match; | ||
| std::regex_search(str, match, reg); | ||
|
|
||
| if ( !match.empty() ) { |
There was a problem hiding this comment.
Minor nit: in the rest of the code, there are not spaces between parentheses and conditions (see for example at L38 below). I would use the same convention for consistency
test/test.cpp
Outdated
| Quantity t2(1, "h", Units::Time); | ||
| Quantity t3("60min", Units::Time); | ||
| Quantity t4("3601 sec", Units::Time); | ||
| t1.fromString("2days"); | ||
| //t1.fromString("2days"); |
There was a problem hiding this comment.
Is commenting this out intended? Without this we are no longer testing anywhere the fromString method
Makefile
Outdated
| rm $1; \ | ||
| exit 1; \ |
There was a problem hiding this comment.
Minor nit: not sure if intended but by adding space in front of these two lines makes them no longer aligned with the echo "OK"; \ instruction above. Was this change intended?
test/test.cpp
Outdated
| @@ -302,6 +302,8 @@ void testEdge() { | |||
| cout<<"Edge test: "; | |||
|
|
|||
| Edge e; | |||
|
|
|||
| e.setBufferSize(Quantity("1kB", Units::Data)); | |||
There was a problem hiding this comment.
Minor nit like my comment above about using tabs instead of spaces
- fixed spaces to tabs in a few spots (tabs project default) - fixed spacing conventions around if conditions. - fixed spaces in shell script - not the same length as surrounding code - reenabled a test after I commented it out to get changes working
|
Good catches all. I'm sorry I made you go through the detailed review. I'll remember the project conventions going forward! |
This change: