asinha94
commented
7 years ago So Good job so far, you did manage to get most of the way there, I just have a couple of comments.
General Code Structure
So you see above the 2nd while loop you have big list of variables (Line 21-31). those are called flag variables and are generally considered not great practice for a few reasons. The main one being that flags keep track of some kind of state and as your program grows, so do the number of states and the ability to debug gets really difficult. In this case our program probably won't be extended that much, but even as-is we can both see it's a bit hard to debug.
So I think it's probably worth re-writing the structure of a few sections/applying a bit different logic.
New Structure (inside the first while loop)
Flags
So we can't get rid of all flag variables but I think we should limit it to 2-3 max. Those being beginning and end (which we will use as iterators which you can do a lot more with).
auto begin = str.begin();
auto end = str.end();
Structure (i.e. what order we should parse in)
- At first I think we should check for the "#" character. If it's found, set the end iterator to be that value
end = begin + find; - Generate the substring with the string constructor (a cool trick I just found)
std::string substring(begin, end) - After that test if the length of the substr is too small (less than 4 characters). Because the smallest valid expression possible is
a;a;anything smaller is either empty or invalid. If it is invalidfprintf()an error message thencontinue(i.e skip to the next while loop iteration). - Use the
countfunction included in<algorithm>to find the number of semi-colonsint num_semicolons = std::count(begin, end)if the number of semicolons is not equal to 2, then there's an error, sofprintfandcontinueagain - Find the positions of both semicolons as iterators (now that we know for sure that there is only 2)
- Create the 2 different strings with semicolons removed (which still have whitespace)
- Remove the leading and trailing whitespace from both strings. You can do it however you we're doing it before. Replace any other spaces with underscores
- And finally store the values into the map (which is defined outside the loop) and found in
<map>std::map<std::string, std::string> configs;configs["key"]=key;configs["value"]=value;
So doing it that way eliminates all those variables (expect begin and end) and the need for 2nd loop. I may have forgotten a few things, so feel free to modify this structure as needed
Pedantic stuff
- So the standard C/C++ bracket style is to keep the first brace on the same line. Also else-statements should be on the same line as the closing brace (for the if)
if (true) { //stuff } else { // more stuff }This I feel is a bit easier to read than this
if () {
}
else {
}
because the 2nd else can be condfused for an if, if you are skimming
- Make sure you're indentation is all in-sync e.g line 20-21 are indented for some reason.
- So you kinda have a spaces mixed up with tabs there. It doens't really matter which ones you use, but Im not a fan of tabs. They always look weird in the editors I use. So if you could replace them with spaces that would be appreciated. You can do that in gedit by `search->replace` and searching for `\t` and replacing with a space
# Other Stuff
- Make sure you compile with c++11
- Store everything as a string. dont try and convert it.
- I wasn't sure about which map would be best, but if you want to then you can research on it. What we are looking for is something that has fast reads more than anything else. I think hashmap has the best running time `O(1)` but it doesn't really matter
adaln
Draft of config parser. Problems with finding ";" (lines 45-94)