Expression: vector subscript out of range [Debug Assertion Fail]

[Nwb]

I'm using Microsoft Visual Studios 2017. I have tried this code for ours but I donno what's wrong!

Here's the error pic:


The code works fine without the

bool equal_to_operator_flag = false;

So that seems to be the fault over here but I have no clue why.

Here's the full code:

#include <conio.h>
#include <iostream>
#include <ctype.h>
/* For character comparisions */
#include <vector>
#include <string>
#include <windows.h>
/* For Sleep.. :D */
#include <algorithm>
/* For replace */
#include <sstream>
/* For splitting input to terms */


int main() {

    using namespace std; 
    using std::cout;
    //  Temporary 
    
    /*-------------------------------------------------------------------------*/

    cout << " Please type an expression \n    (each term has one variable, alpha-numerical only, \n      operations and variables separated by spaces)\n\n";

    string input_expression;
    getline(cin, input_expression);
    // input

    {
        int length = (int)input_expression.length();
        for (int i = 0; i < length; i++) {
            if (input_expression[i] == '+' || input_expression[i] == '/' || input_expression[i] == '-' || input_expression[i] == '*' || input_expression[i] == '^' || input_expression[i] == '%') {
                input_expression.insert(i, " ");
                input_expression.insert(i + 2, " ");
            }
        }
    }

    std::replace(input_expression.begin(), input_expression.end(), '\t', ' ');
    //Trims Tabspaces

    std::string::size_type pos;
    while ((pos = input_expression.find("  ")) != std::string::npos)
    {
        input_expression.replace(pos, 2, " ");
    }// Trims multiple white spaces to just one

    cout << "You entered the expression: "<<input_expression<<endl<<endl;

    /*-------------------------------------------------------------------------*/

    string buf;
    stringstream ss(input_expression);

    vector <string> terms;
    vector <string> numeric;
    vector <string> alpha_numeric;
    vector <string> term_operator;
    vector <string> invalid;
    vector <string> sorted_terms;
    vector <int> term_type;


    // 0 - alpha-numeric
    // 1 - numeric
    // 2 - ^
    // % - 3
    // ( - 4, ) - 5
    // / - 6
    // * - 7
    // + - 8
    // - - 9
    // = - 10



    while (ss >> buf)
        terms.push_back(buf);

    cout << "Terms: ";
    for (int i = 0; i < (int)terms.size(); i++)
        cout << terms[i] << " | ";

    cout << "\n\n\n";

    /*-------------------------------------------------------------------------*/

    // input = 
    //              -A 636 / a5 6& -24 -5.5 -6a 9! 5..4 = .5 AA5 =  5aa 53a53 53a a3 =

    bool equal_to_operator_flag = false;

    for (int i = 0; i < (int) terms.size() ; i++) {


            bool term_operator_flag = false;
            bool numeric_flag = true;
            bool valid_flag = true;
            int decimal = 0;
            int alpha = 0;
            bool alpha_taken_flag = false;
//            bool equal_to_operator_flag = false;
            

        string term = (string) terms[i];

    
        // ^ - 2
        // % - 3
        // / - 6
        // * - 7
        // + - 8
        // - - 9
        // = - 10

        if (term == "=") {
            term_operator_flag = true;
            if(equal_to_operator_flag)
                invalid.push_back(terms[i]);
            else {
                term_operator.push_back(terms[i]);
                term_type.push_back(10);
                equal_to_operator_flag = true;
            }
        }
        else if (term == "^") {
            term_operator.push_back(terms[i]);
            term_type.push_back(2);
            term_operator_flag = true;
        }
        else if (term == "%") {
            term_operator.push_back(terms[i]);
            term_type.push_back(3);
            term_operator_flag = true;
        }
        else if (term == "/") {
            term_operator.push_back(terms[i]);
            term_type.push_back(6);
            term_operator_flag = true;
        }
        else if (term == "*") {
            term_operator.push_back(terms[i]);
            term_type.push_back(7);
            term_operator_flag = true;
        }
        else if (term == "+") {
            term_operator.push_back(terms[i]);
            term_type.push_back(8);
            term_operator_flag = true;
        }
        else if (term == "-") {
            term_operator.push_back(terms[i]);
            term_type.push_back(9);
            term_operator_flag = true;
        }

        for (int i = 0; i < (int) term.length(); i++) {

            if (i == 0) {
                if (!(isdigit(term[0]))) {     // If it's not a digit  
                    if (term[0] != '-')        // If first value isn't '-'
                        if ((term[0] != '.'))  // If first value isn't '.'
                            numeric_flag = false;
                        else
                            decimal++;
                }
                if (!(isalnum(term[0])))     // If it's not alpha-numerical
                    if (term[0] != '-')      // if first value isn't '-'
                        valid_flag = false;

                if (isalpha(term[0])) 
                    alpha++;
            }
            else {
                if (isalpha(term[i])) {        // If it's an alphabet
                    alpha++;
                    alpha_taken_flag = true;
                }
                if ((isdigit(term[i])) && alpha_taken_flag)
                    valid_flag = false;

                if (!(isdigit(term[i]))) {     // If it's not a digit  
                    if ((term[i] != '.'))      // If it isn't '.'
                        numeric_flag = false;
                    else
                        decimal++;

                    if (decimal > 1)      // If more than one decimal
                        numeric_flag = false; 
                }
                if (!(isalnum(term[i])))     // If it's not alpha-numerical
                    valid_flag = false;
                else if (alpha > 1)          // If there's more than one alphabet in the term
                    valid_flag = false; 

            }
        }

        if (term_operator_flag) {
            //term_operator.push_back(terms[i]);
            //if(terms[i]== "(")
            term_operator_flag = false;
        }
        else if (numeric_flag) {
            numeric.push_back(terms[i]);
            term_type.push_back(1);
        }
        else if (valid_flag) {
            alpha_numeric.push_back(terms[i]);
            term_type.push_back(0);
        }
        else
            invalid.push_back(terms[i]);

    }
    
    cout << endl << "Operator Terms:" << endl << "\t";

    for (int i = 0; i < (int)term_operator.size(); i++) {
        cout << term_operator[i] << " | ";
    }

    cout << endl<< "Numeric Terms:"<<endl<<"\t";

    for (int i = 0; i < (int)numeric.size(); i++) {
        cout << numeric[i]<<" | ";
    }

    cout << endl << "Alpha-Numeric Terms:" << endl << "\t";

    for (int i = 0; i < (int)alpha_numeric.size(); i++) {
        cout << alpha_numeric[i] << " | ";
    }

    cout << endl << "Invalid Terms:" << endl<<"\t";

    for (int i = 0; i < (int)invalid.size(); i++) {
        cout << invalid[i] << " | ";
    }

    cout << endl << endl << "Inputted Terms were:" << endl<<"\t";

    for (int i = 0; i < (int)terms.size(); i++) {
        cout << terms[i] << " ";
    }

    cout << endl << endl;
    cout << "Terms after sorting:" << endl;

    for (int i = 0; i < (int)terms.size(); i++) 
        if ((std::find(numeric.begin(), numeric.end(), terms[i]) != numeric.end()) || (std::find(alpha_numeric.begin(), alpha_numeric.end(), terms[i]) != alpha_numeric.end()) || (std::find(term_operator.begin(), term_operator.end(), terms[i]) != term_operator.end()))
            sorted_terms.push_back(terms[i]);

    for (int i = 0; i < (int) sorted_terms.size(); i++) {
        cout << sorted_terms[i] << "(" << term_type[i] << ") | ";
    }

    _getch();
    return 0;
}

[Salem]

1. You see where it says "press retry to debug the application"?
Press it!

You should then drop into the debugger, from where you can navigate the call stack to locate the actual line of your code causing the problem.

And from that point, you can examine variables to determine which vector, and which subscript is causing the problem.


2. Read this before you decide to write another program with a 250+ line main() function.


> The code works fine without the
> bool equal_to_operator_flag = false;
Which one?
The one on line 91 or line 102?

Because if you have both variables live at the same time, then you're in for all sorts of fun.

[Guest]

The error points out that your program tried to access a std::vector by an index larger than the size of the container. The cause might not be quick to locate, but I would suggest you take it step by step. Given the size of your main function, it would also help to refactor the code a bit where feasible and limit indentation levels.

A few random tips:

Instead of casting to int:

for (int i = 0; i < (int)terms.size(); i++)
        cout << terms[i] << " | ";

...you should loop with the size type directly (since that's what terms[i] expects as well) or use a range based for loop:

for (const auto& t : terms)
    cout << t << " | ";

Likewise, don't cast where it's entirely superfluous, like:

string term = (string) terms[i]; // terms[i] returns a std::string reference already

In your case, switching to range-for loops will make this look a lot cleaner already.

If you don't plan to modify a container, prefer the cbegin() and cend() methods over their non-const counterparts.

Avoid C header files, especially the non-standard conio.h. Use their C++ equivalent, e.g. cctype if necessary. To make the console wait for input, try:

std::cin.get(); // instead of _getch();

[Nwb]

Thanks a lot to everybody who replied. It was a problem with one of the for-statements. Also thanks a lot for the tips.. I really appreciate it!

Adrian, thanks a lot for telling me about range based for-loops, I never knew about them! But can you explain why in (const& t : terms) const auto& is used instead of "auto"? (I don't know about pointers). Why is const identifier used?

Also I have seen people using auto&& instead of just auto. And you used auto& What's the difference?

[Nwb]

Also I casted terms.size to int because the compiler told me that there was a mismatch of data-types..

[jimblumberg]

But what you should have done is use a size_t for i in that loop, not cast the return value from the size() function.

And if you insist on casting use the C++ style casts static_cast<int>(variable) instead of the C style cast.

Also casting should be the last resort, always try to use the correct type of variables for the operation first.

[laserlight]

Adrian, thanks a lot for telling me about range based for-loops, I never knew about them! But can you explain why in (const& t : terms) const auto& is used instead of "auto"? (I don't know about pointers). Why is const identifier used?

If you just used auto, you would be making a copy of the corresponding element on each iteration. That is a rather unnecessary copy, that's why you would use const auto& so that the identifier would be a const lvalue reference to the corresponding element, hence avoiding the copies.

Also I have seen people using auto&& instead of just auto. And you used auto& What's the difference?

auto&& would specify that the identifier would be a universal reference to the corresponding element that will behave as an lvalue reference in this case, i.e., if you do make a modification in the loop body, it will affect the corresponding element itself.

Also I casted terms.size to int because the compiler told me that there was a mismatch of data-types..

size() return a size_type (as in std::vector<std::string>::size_type), which is an unsigned integer type, but int is a signed integer type. This can be problematic, hence the warning. As Adrian mentioned, the right approach was to use size_type to begin with, whether it be by spelling it out or by other means, e.g.,

for (decltype(terms.size()) i = 0; i < terms.size(); ++i)

[Nwb]

If you just used auto, you would be making a copy of the corresponding element on each iteration. That is a rather unnecessary copy, that's why you would use const auto& so that the identifier would be a const lvalue reference to the corresponding element, hence avoiding the copies.


auto&& would specify that the identifier would be a universal reference to the corresponding element that will behave as an lvalue reference in this case, i.e., if you do make a modification in the loop body, it will affect the corresponding element itself.


size() return a size_type (as in std::vector<std::string>::size_type), which is an unsigned integer type, but int is a signed integer type. This can be problematic, hence the warning. As Adrian mentioned, the right approach was to use size_type to begin with, whether it be by spelling it out or by other means, e.g.,

for (decltype(terms.size()) i = 0; i < terms.size(); ++i)

Oh ok. And what about making the iterator unsigned?

[whiteflags]

Iterators aren't numbers, they don't need to be unsigned.

For example if I wanted this to use iterators, I might write:

 for (auto it = terms.rbegin(); *it != terms.rend(); ++it)

The type for the iterator is probably not correct, but whatever term's type is, you'd just put that in front of ::reverse_iterator (if you want to write it).
Of course other loops you could make are going to be pretty similar. The most different it could get is range-for.

 for (auto term : terms) // assign terms[0], terms[1], etc to term and go

[Nwb]

Iterators aren't numbers, they don't need to be unsigned.

For example if I wanted this to use iterators, I might write:

 for (auto it = terms.rbegin(); *it != terms.rend(); ++it)

The type for the iterator is probably not correct, but whatever term's type is, you'd just put that in front of ::reverse_iterator (if you want to write it).
Of course other loops you could make are going to be pretty similar. The most different it could get is range-for.

 for (auto term : terms) // assign terms[0], terms[1], etc to term and go

Your first example gives the following error:

    for (auto it = factors.rbegin(); *it != factors.rend(); ++it)

error: binary '!=': no operator found which takes a left-hand operand of type 'std::vector<float,std::allocator<_Ty>>'(or there is no acceptable conversion)

I notice that * is the fault here. Anyways why are we using a rbegin and rend instead of a regular begin and end?


factors is a 2D vector. Why isn't this working:

for (const auto& x : factors)
        for (const auto& y : factors[x])
            cout << y << "\t";

I get the errors:
-> 'y': undeclared identifier (idk why)
-> binary '[': no operator found which takes a right-hand operand of type 'const std::vector<float, std::allocator<_Ty>>'(or there is no acceptable conversion)
-> no operator "[]" matches these operands


So const auto& is the best? When is auto&& used over const auto&?


Also is using .begin() and .end() more efficient than using [0] and .size()?


Thanks!

[whiteflags]

Your first example gives the following error:

    for (auto it = factors.rbegin(); *it != factors.rend(); ++it)

error: binary '!=': no operator found which takes a left-hand operand of type 'std::vector<float,std::allocator<_Ty>>'(or there is no acceptable conversion)

I notice that * is the fault here.

My bad. Your analysis is correct, however.

Anyways why are we using a rbegin and rend instead of a regular begin and end?

I got the idea from laserlight, who posted a loop going in reverse.

As Adrian mentioned, the right approach was to use size_type to begin with, whether it be by spelling it out or by other means, e.g.,

for (decltype(terms.size()) i = 0; i < terms.size(); ++i)

Laserlight's loop uses subscripts and goes in reverse, my loop uses iterators and goes in reverse. ::rbegin() and ::rend() are the correct functions to call in order to write that loop. I probably should have made it clear whose example I was using. I'm sorry I didn't.

factors is a 2D vector. Why isn't this working:

for (const auto& x : factors)
        for (const auto& y : factors[x])
            cout << y << "\t";

You need practice with range-for, I guess. The outer loop assigns a certain member of factor to x, which is in itself another sequence, so you should be able to use x in the internal loop.

Try:

#include <iostream>
#include <vector>

using namespace std;

int main()
{
    vector<vector<int>> factors{{1,2,3,4,5}, {5,4,3,2,1}, {3,2,4,1,5}};
    for (const auto& x : factors) {
        for (const auto& y : x) {
            cout << y << '\t';
        }
        cout << endl;
    }
}

C:\Users\jk\Desktop>g++ -std=c++0x foo.cpp

C:\Users\jk\Desktop>a.exe
1       2       3       4       5
5       4       3       2       1
3       2       4       1       5

So const auto& is the best? When is auto&& used over const auto&?

Do you need an rvalue or not? Read laserlight's comment again and see if you can glean what auto&& means. If you need what auto&& does, then it's the best. If not then there's nothing wrong with something else.

Also is using .begin() and .end() more efficient than using [0] and .size()?

I think it comes down to personal preference more than anything, when you get to make a choice. In reality, you probably won't have a choice most of the time because there is another function you want to call that needs a certain set of arguments, and it is clear whether you should use iterators or subscripts. For example, standard algorithms always want to use iterators.

[laserlight]

I got the idea from laserlight, who posted a loop going in reverse.

No, that loop loops forward.