Attributes Parsing in C++ Challenge

Question

Link to task

Similar question to same challenge

Attribute Parser

This challenge works with a custom-designed markup language HRML. In HRML, each element consists of a starting and ending tag, and there are attributes associated with each tag. Only starting tags can have attributes. We can call an attribute by referencing the tag, followed by a tilde, '~' and the name of the attribute. The tags may also be nested.

The opening tags follow the format:

<tag-name attribute1-name = "value1" attribute2-name = "value2" ...>

The closing tags follow the format:

The attributes are referenced as:

tag1~value 
tag1.tag2~name

Given the source code in HRML format consisting of lines, answer

queries. For each query, print the value of the attribute specified. Print "Not Found!" if the attribute does not exist.

Example

HRML listing
<tag1 value = "value">
<tag2 name = "name">
<tag3 another="another" final="final">
</tag3>
</tag2>
</tag1>
Queries
tag1~value
tag1.tag2.tag3~name
tag1.tag2~value

Here, tag2 is nested within tag1, so attributes of tag2 are accessed as tag1.tag2~<attribute>. Results of the queries are:

Query Value
tag1~value "value"
tag1.tag2.tag3~name "Not Found!"
tag1.tag2.tag3~final "final"

Input Format

The first line consists of two space separated integers, and . specifies the number of lines in the HRML source program.

specifies the number of queries.

The following

lines consist of either an opening tag with zero or more attributes or a closing tag. There is a space after the tag-name, attribute-name, '=' and value.There is no space after the last value. If there are no attributes there is no space after tag name.

queries follow. Each query consists of string that references an attribute in the source program.More formally, each query is of the form ~ where and

are valid tags in the input.

Constraints

Each line in the source program contains, at most, characters. Every reference to the attributes in the queries contains at most characters. All tag names are unique and the HRML source program is logically correct, i.e. valid nesting. A tag can may have no attributes.

Output Format

Print the value of the attribute for each query. Print "Not Found!" without quotes if the attribute does not exist.

Sample Input

4 3
<tag1 value = "HelloWorld">
<tag2 name = "Name1">
</tag2>
</tag1>
tag1.tag2~name
tag1~name
tag1~value

Sample Output

Name1
Not Found!
HelloWorld

Question

How is my code? It passed all cases. Is it not to complex? I think that keeping separate functions is easier to maintain. On the other hand I saw peoples doing it in one loop, but it may be tricky if problems would be bigger.

Second smaller question: is it ok to clear string in function?

Code

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
#include <map>
using namespace std;
map<string, string> maptags;
vector<string> v_tags;
int read_tag_name(string *key, char **ptr){ 
 *key = "";
 while(true){
 // cout << "r1 checking: " << **ptr << '\n';
 if (**ptr == '>'){
 return -1;
 } else if (**ptr == ' '){
 (*ptr)++;
 return 0;
 }
 *key = *key + **ptr;
 (*ptr)++;
 }
 return 0;
}
int read_attr_name(string *key, char **ptr){
 *key = "";
 while(true){
 if (**ptr == '>'){
 return -1;
 } else if (**ptr == ' ' || **ptr == '='){
 (*ptr)++;
 return 0;
 }
 *key = *key + **ptr;
 (*ptr)++;
 }
 return 0;
}
int read_attr_val(string *key, char **ptr){
 *key = "";
 bool looking4start = true;
 
 while(true){
 if (looking4start && **ptr == '>'){
 return -1;
 } else if (looking4start && **ptr == ' '){
 (*ptr)++;
 continue;
 } else if (looking4start && **ptr == '"'){
 looking4start = false;
 (*ptr)++;
 continue;
 } else if (!looking4start && **ptr == '"'){
 (*ptr)++;
 if(**ptr == ' '){
 (*ptr)++;
 }
 return 0;
 }
 if (!looking4start){
 *key = *key + **ptr;
 }
 (*ptr)++;
 }
 return 0;
}
void process_lines(size_t N){
 // READ New tags. Endings, Attributes.
 // Task has only valid nestings.
 // Tags can be empty.
 
 string line; 
 string cur_key="";
 string cur_tag="", temp_attr="", temp_val=""; 
 
 char* index;
 
 for(int i=0; i<N; i++){ 
 getline(cin, line);
 index = &line[1];
 
 if (line[1] == '/'){
 // Closing tag
 
 if (v_tags.size()>1){
 // Deleting with dot
 cur_key.erase(cur_key.size()-v_tags[v_tags.size()-1].length()-1);
 v_tags.pop_back();
 
 } else {
 cur_key = "";
 v_tags.clear();
 }
 
 
 
 } else {
 // New tag case
 
 if (read_tag_name(&cur_tag, &index) < 0){ 
 v_tags.push_back(cur_tag);
 if (cur_key.length()< 1){
 cur_key = cur_tag;
 } else {
 cur_key += '.' + cur_tag;
 }
 continue;
 };
 
 v_tags.push_back(cur_tag);
 if (cur_key.length()<=0){
 cur_key += cur_tag; 
 } else {
 cur_key += '.' + cur_tag;
 }
 
 while(true){
 
 if(read_attr_name(&temp_attr, &index)<0){
 break;
 }
 // cout << "Pointer2 at: " << *index << "\n";
 if (read_attr_val(&temp_val, &index)<0){
 maptags[cur_key+"~"+temp_attr] = temp_val; 
 break;
 }
 maptags[cur_key+"~"+temp_attr] = temp_val; 
 } 
 } 
 } 
}
void print_count(string key){ 
 cout << key << ": " << maptags.count(key) << std::endl;
}
void process_queries(int N){
 string query;
 for(int i=0; i<N; i++){
 getline(cin, query);
 if (maptags.count(query)>0){
 cout << maptags[query] << '\n';
 } else{
 cout << "Not Found!" << '\n';
 }
 }
}
int main() { 
 int N, Q;
 cin >> N >> Q >> std::ws;
 
 process_lines(N);
 process_queries(Q); 
 
 return 0;
}
```

• Rish: Please include the problem statement in the post. Hyperlinks tend to go stale.
• Tamoghna Chowdhury: This is just C code with vector and map thrown in.
• Code Gorilla: @TamoghnaChowdhury - its not is has // in, that makes it C++ :D.
• Tamoghna Chowdhury: // has been in C since C99 @CodeGorilla

Answer 1

When I look at a post, I skim down to the code and see what I think about it, before reading the post explaining what it does and why its posted here. I skimmed through your code and I haven't read the post. I'm sorry to have to say that I think that the way you code is written means even if it does exactly what it is required to do it is still bad code. I know thats not a nice thing to say and I'm sorry, but please let me explain why I think this.

Any code that has a global using namespace std; is not going to be good. It negates the purpose of name spaces and is a bad sign.

There are 3 lines of comments and although you know what the code does now I bet you won't know in a years time.

Declaring multiple variables on a single line makes maintenance of code much harder.

You are passing values that the user has entered to functions without checking them. what happens if N is -42?

Using single letters as variable names is the same as not adding comments, the names of variables help the reader understand the code.

while(true) always raises flags. Its a symptom of really badly designed code. As are the random return and continue statements, I'm certain that the loops could be much more elegant if you tried.

getline What happens if you read nothing and then look at line[1]?

Passing pointers to std::strings indicates something is wrong in code with such a simple task.

Sorry I can't comment on how good your solution is, because I don't want to read and understand the code, I just want to hide from it.

What you could do is:

1. std::string is much more than a container for characters. Have a look and see if you could utilize some of the functions.
2. Look into input/parameter validation
3. Comment/document your code so people understand what is happening

• Grzegorz Krug: Could you explain more on strings container? I have seen code which used substr after string iteration. Is it redundant and makes string iterate twice? or maybe its that function is o(1) and storing letter one by one is silly as you said.
• Code Gorilla: Sorry @GrzegorzKrug, your code is written in such a way that I don't understand what its trying to achieve. I think you are tokeninsing the input string and then parsing the tokens. You seem to have tried to write everything yourself, which is almost always a mistake. I would go back to your written explanation of what your code is meant to do and try and match std functions to it. If you want to do it in the style you have then strtok would be a start, if you want to use std::string then find and substr are worth looking at.