Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here you'll find the problem: http://uva.onlinejudge.org/index.php?option=com_onlinejudge&Itemid=8&page=show_problem&problem=614

#include<iostream>
#include<stack>
#include<string>
#include<cstdio>

/*farnaws,C++,673,08/12/2012*/

using namespace std;


string verifier(string input_line)
{

    stack <char> braces;
    int flag=0, count_left_first_brace=0,count_left_third_brace=0;
    for(int i=0; i<input_line.size(); i++)
    {

        if(input_line[i]=='(' || input_line[i]=='[')
        {
            braces.push(input_line[i]);
            if(input_line[i]=='(')
            {       
                count_left_first_brace++;
            }
            else if(input_line[i]=='[')
            {
                count_left_third_brace++;
            }

        }
        else if(input_line[i]==')' || input_line[i]==']')
        {
            if(!(braces.empty()==true))
            {
                if((input_line[i]==')' && braces.top()=='(') || (input_line[i]==']' && braces.top()=='[') )
                {
                    braces.pop();   
                }

            }

            if(input_line[i]==')')
            {       
                count_left_first_brace--;
            }
            else if(input_line[i]==']')
            {
                count_left_third_brace--;
            }
        }

    }
    if((braces.size()==0 && count_left_third_brace>=0 && count_left_first_brace>=0) || input_line.empty())
    {
         return "Yes";      
    }
    else
    {   
        return "No";
    }

}
int main()
{

   char line[128];
   int limit;
   cin>>limit;
   getchar();
   for(int i=0; i<limit; i++)
   {
            cin.getline(line,sizeof(line));
            string str=line;
            cout<<verifier(str)<<endl;
   }

   return 0;


}
share|improve this question

closed as off topic by ANeves, Brian Reichle, Jeff Vanzella, Glenn Rogers, Anton Golov Dec 11 '12 at 22:28

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined by the community. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about reopening questions here.If this question can be reworded to fit the rules in the help center, please edit the question.

add comment

1 Answer 1

Here's a style review, with a glance at efficiency. Not going to check for correctness, as that's outside the scope of the site. I'll start from main.

char line[128];

Why not use an std::string here? You're creating one later anyway; using it directly and passing by const reference would not be any slower.

getchar();

Either use streams, or use C-style IO; don't mix the two. Besides, streams have a perfectly good function to do this.

cin.getline(line, sizeof(line));

Don't do this. Do this instead:

std::getline(std::cin, line); // here, line should be an std::string

I would also not have verifier return a string; instead:

std::cout << (verifier(str) ? "Yes" : "No") << '\n';

(You have no reason to flush here.)

Moving onto the verifier function itself. A range-based for loop would be much more convenient:

for (char c : input_line) {
    // logic
}

The flag variable is unused, as far as I can tell.

The if(!(braces.empty()==true)) could much better be written as

if (!braces.empty())

Your count_left_third_brace and count_right_third_brace variables are not necessary; you can determine whether the code is right based purely on the contents of the stack.

The function boils down to:

bool verifier(std::string const& str) {
    std::stack<char> braces;
    for (char c : str) {
        switch (c) {
            case '(':
            case '[':
                braces.push(c);
                break;
            case ')':
                if (braces.empty() || braces.top() != '(')
                    return false;
                braces.pop();
                break;
            case '[':
                if (braces.empty() || braces.top() != ']')
                    return false;
                braces.pop();
                break;
        }
    }
    return true;
}

This is mostly equivalent to the existing code, and is probably closer to the intent.

share|improve this answer
    
"Not going to check for correctness, as that's outside the scope of the site." Asking for code correctness is on-topic, as long as the code initially works. –  seand Dec 11 '12 at 20:00
    
@seand: From your link, "Correctness in unanticipated cases". Seeing as the OP has already discovered that there is some error, that error is off topic. –  Anton Golov Dec 11 '12 at 21:59
    
I have discovered the error and also solved the problem, thank you guys for your effort. –  MiNdFrEaK Dec 11 '12 at 22:12
add comment

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