4
\$\begingroup\$

This is a challenge question from hackerrank.com.

We have defined our own 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:

< /tag-name >

For example:

<tag1 value = "HelloWorld">
<tag2 name = "Name1">
</tag2>
</tag1>

The attributes are referenced as:

tag1~value  
tag1.tag2~name

You are given the source code in HRML format consisting of N lines. You have to answer Q queries. Each query asks you to print the value of the attribute specified. Print "Not Found!" if there isn't any such attribute.

Input Format

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

The following N lines consist of either an opening tag with zero or more attributes or a closing tag.

Q queries follow. Each query consists of string that references an attribute in the source program.

Constraints

1 ≤ N ≤ 20
1 ≤ Q ≤ 20

Each line in the source program contains, at max, 200 characters.

Every reference to the attributes in the Q queries contains at max 200 characters.

All tag names are unique.

Output Format

Print the value of the attribute for each query. Print "Not Found!" without quotes if there is no such attribute in the source program.

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

Please review my code and let me know your suggestions.

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
#include <sstream>
#include <map>
#include <iterator>



int main() {
    std::string first;
    std::getline(std::cin, first);
    std::istringstream iss(first);
    std::vector<int> tokens1;
    std::copy(std::istream_iterator<int> (iss),
                 std::istream_iterator<int>(),
                 std::back_inserter(tokens1));

    int n = tokens1[0], q=tokens1[1];
    std::string source;
    std::map<std::string, std::string> tagValue; 
    std::string tagKey;

    while( n > 0)
    {  
        std::string input;
        std::getline(std::cin, input);

        std::istringstream iss(input);
        std::vector<std::string> tokens;
        std::copy(std::istream_iterator<std::string> (iss), 
                   std::istream_iterator<std::string>(),
                   std::back_inserter(tokens));

        if( tokens.size() >=1)
        {
            std::string tag = tokens[0];
            if( tag[0] == '<' && tag[1] != '/')
            {

                if( tag[tag.size()-1]== '>')
                     tagKey.empty() ? tagKey = tag.substr(1,tag.size()-2):
                                      tagKey = tagKey+"."+tag.substr(1, tag.size()-2);
                else
                     tagKey.empty() ? tagKey = tag.substr(1):
                                      tagKey = tagKey+"."+tag.substr(1);  
            }
            if( tag[0] == '<' && tag[1] == '/')
            {
                auto found = tagKey.find_last_of(".");
                if( found != std::string::npos)
                   tagKey.erase(tagKey.begin()+found, tagKey.end());
                else
                   tagKey="";
            }            
        }
        if(tokens.size() > 1 )
        {  
            int i = 1;
            while( i+2 < tokens.size() )
            {        
                std::string name = tokens[i];
                std::string value = tokens[i+2];
                if( value[value.size()-1]== '>')
                    tagValue[tagKey+"~"+name] = value.substr(1, value.length()-3);
                else
                    tagValue[tagKey+"~"+name] = value.substr(1, value.length()-2);
                i+=3;
            }
        }
        --n;

    }

    std::string input1;
    while(  std::getline(std::cin, input1)) 
    {

        if( q < 0)
            break;
         std::istringstream iss(input1);
         std::vector<std::string> tokens;
         std::copy(std::istream_iterator<std::string> (iss), 
                   std::istream_iterator<std::string>(),
                   std::back_inserter(tokens));

         (tagValue.find(tokens[0]) != tagValue.end()) ? 
                                      std::cout<<tagValue[tokens[0]]<<"\n" :
                                      std::cout<<"Not Found!\n";
       --q; 
    }
}
\$\endgroup\$
  • \$\begingroup\$ So a question that reinvents XPath. \$\endgroup\$ – Martin York Apr 8 '17 at 4:53
  • \$\begingroup\$ It looks like you could easily write a SGML Declaration for that (or perhaps even use the Reference Concrete Syntax), then use one of the standard libraries... \$\endgroup\$ – Toby Speight May 4 '17 at 12:43
4
\$\begingroup\$

Here are some ideas that may help you improve your code.

Use functions

Right now this code is all one big block of code in main. This makes it more difficult to read and understand than if functions were used.

Simplify your code

Consider this sequence of code to read two integers:

std::string first;
std::getline(std::cin, first);
std::istringstream iss(first);
std::vector<int> tokens1;
std::copy(std::istream_iterator<int> (iss),
             std::istream_iterator<int>(),
             std::back_inserter(tokens1));

int n = tokens1[0], q=tokens1[1];

Now consider this version:

int n, q;
std::cin >> n >> q;

Which is easier to read and understand? It's true they don't do the exact same thing, but I suspect the differences don't much matter in this case.

Eliminate unused variables

Unused variables are a sign of poor quality code, and you don't want to write poor quality code. In this code, source is unused. Your compiler may be smart enough to tell you about this if you ask it nicely.

Don't #include headers that aren't needed

This code includes <cmath> and <cstdio> but neither are actually used. Those lines should be deleted.

Use appropriate data structures

The tagKey variable is a std::string and keeps the current tag stack as a '.' separated string. However this gets messy and complex because "popping" a tag from the end means looking for that delimiter and then erasing just that bit. I'd be inclined to create a class for that to encapsulate this complexity. Here's one way to do that:

class StringStack : public std::deque<std::string> {
public:
    std::string asString() const {
        auto it{cbegin()};
        std::string str{*it};
        for (++it; it != cend(); ++it) {
            str += "." + *it;
        }
        return str;
    }
};

Use a state machine

The logic of this code could be expressed as a state machine. If that were done, one could process the stream "on the fly" character at a time with little difficulty.

Prefer unordered_map to map

There's not really any reason that this code needs a map instead of an unordered_map and one generally gets a performance increase using the unordered version.

An example

Using the suggestions above, my rewritten version of main looks like this:

int main() {
    constexpr std::size_t maxlinelen{200};

    int n, q;
    std::cin >> n >> q;
    std::cin.ignore(maxlinelen, '\n');

    auto tagValue{fetchTags(std::cin, n)};

    for(std::string query; q > 0 && std::getline(std::cin, query); --q) {
        auto search = tagValue.find(query);
        if (search == tagValue.end()) {
            std::cout << "Not Found!\n";
        } else {
            std::cout << search->second << '\n';
        }
    }
}

I implemented a fetchTags() routine using a state machine and returning a std::unordered_map<std::string, std::string> but rather than spoil the fun, I'll leave it to you to implement your own.

\$\endgroup\$
-2
\$\begingroup\$

To get sample output I added vector<string> results at the beginning.

Then I changed your while loop to this:

 while(q!=0)
    {
        std::getline(std::cin, input1);
         std::istringstream iss(input1);
         std::vector<std::string> tokens;
         std::copy(std::istream_iterator<std::string> (iss),
                   std::istream_iterator<std::string>(),
                   std::back_inserter(tokens));

         (tagValue.find(tokens[0]) != tagValue.end()) ?
                                      results.push_back(tagValue[tokens[0]]+"\n") :
                                      results.push_back("Not Found!\n");
       --q;
    }

Finally, added a for loop for output:

for(auto &a: results){
        cout << a;
    }
\$\endgroup\$
  • 4
    \$\begingroup\$ Please explain why you think your modified while loop is an improvement over the one given in the question. \$\endgroup\$ – Null Jan 4 at 17:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.