C++

Forum

 
Error with vector and calling function

DonnaPin (154)
Hello, I am trying to complete a problem from the programming principles and practice book, in which i need to accept different numbers and different unit measurements and determine what one is the smallest, I also need to only allow legal measurements. I have the code that I think will work, but I am getting 2 errors that I can not figure out. The code is pasted below. The errors I am getting is for the for loop in the legalUnits (line 12) function and it states "No match for operator=" but it is a string so I do not understand why this is happening.

the second issue is line 39 in the line if(legalUnits(units)) and it says "no match for call to std::<Vector>::basic_string <char> then mentions &string, but the functions parameter is a string, and so is the argument, so I do not understand? Could anyone please explain this



1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
 
  // Example program
#include <iostream>
#include <string>
#include <vector>

bool legalUnits(std::string unit)
{
     
    bool isLegal = false;
    std::vector<std::string>::iterator it;
     
    for(it = unit.begin(); it != unit.end(); it++)
    {
        if (*it == unit)
        {
            isLegal = true;   
        }
    }
     
    return isLegal;        
}

int main()
{
    double num;
    double smallest, largest;
    bool firstInput = true;
    std::string unit = " ";
    std::vector<std::string>legalUnits {"cm", "m", "in", "ft"};
    
    
   
    
    while (std::cin >> num)
    {
        std::cout << "Enter a unit of measurement \n";
        std::cin >> unit;
      
      if (legalUnits(unit))
      {
            if (unit == "m")
            {
                num *= 100;        
            }
            else if (unit == "in")
            {
                num *= 2.54;   
            }
            else if(unit == "ft")
            {
                num =  12 * num * 2.54;   
            }
           
            if(firstInput)
            {
                smallest = num;
                largest = num;
                firstInput = false;
            }
      
            if (num < smallest)
            {
                smallest = num;   
                std::cout << smallest << unit << "\n";
            }
            else if(num > largest)
            {
                largest = num;   
                std::cout << largest << unit << "\n";
            }    
             
        std::cout << "Smallest : " << smallest << "cm" << " Largest :" << largest << "cm" << "\n"; 
        
        }
      
    
    }
    
    return 0;
}

Last edited on
Handy Andy (5051)
Hello DonnaPin,

If I understand this the function "legalUnits" should compare the user input to what is contained in a vector.

The problem the function only receives the user input string, but not the vector, so you have created an iterator for a vector, but have no vector to use. Then in the for loop you are trying to set "it" to the beginning of the string that you want to compare to the vector.

You did not mention what page or chapter this problem comes from, so I do not know where to begin to look.

In "main" on line 29 you define std::vector<std::string>legalUnits {"cm", "m", "in", "ft"};, but before that on line 6 you define bool legalUnits(std::string unit).

The problem here with line 39 is that "legalUnits" is a local variable to "main" which overshadows the function of the same name, so compiler is thinking that on line 39 you are trying to use the local variable and not the function.

Consider calling your function CheckLegalUnit. Enough of a difference to keep the 2 separate.

Andy
Handy Andy (5051)
Hello DonnaPin,

I made some changes to your function. Other than changing the name of the function the rest is to give you an idea of what can be done, but not to say that you have to use it.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
bool CheckLlegalUnit(std::vector<std::string> legalUnits, std::string unit)
{
    //bool isLegal = false;
    std::vector<std::string>::iterator it;

    for (it = legalUnits.begin(); it != legalUnits.end(); it++)
    {
        if (*it == unit)
        {
            //isLegal = true;
            return true;
        }
    }

    return false;
}

And for the function call in "main": if (CheckLlegalUnit(legalUnits, unit)). I maid 1 other change in "main" while (std::cout << "\n Enter a number: " && std::cin >> num). This is better than staring at a blank screen wondering what to do.

1 last note: The "cout" statement on line may not always match what the input unit of measure is. not a big deal for a learning experience.

Andy
seeplus (6457)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
 
#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <limits>

struct Units {
	std::string name;
	double mult {};
};

int main()
{
	const std::vector<Units>legalUnits {{"cm", 1.0}, {"m", 100.0}, {"in", 2.54}, {"ft", 12 * 2.54}};

	double smallest {std::numeric_limits<double>::max()}, largest {std::numeric_limits<double>::min()};

	for (double num {}; (std::cout << "Enter quantity: ") && (std::cin >> num); ) {
		std::string unit;
		std::cout << "Enter a unit of measurement:";
		std::cin >> unit;

		const auto fndit {std::find_if(legalUnits.begin(), legalUnits.end(), [&unit](auto legal) {return unit == legal.name; })};

		if (fndit != legalUnits.end()) {
			num *= fndit->mult;

			if (num < smallest)
				smallest = num;

			if (num > largest)
				largest = num;
		} else
			std::cout << "Unit not valid\n";
	}

	std::cout << "Smallest : " << smallest << "cm" << " Largest :" << largest << "cm" << "\n";
}


Rather than using a vector and a search, it would usually be done using a map when you come to that topic.


Enter quantity: 12
Enter a unit of measurement:m
Enter quantity: 13
Enter a unit of measurement:in
Enter quantity: 4
Enter a unit of measurement:ft
Enter quantity: p
Smallest : 33.02cm Largest :1200cm

DonnaPin (154)
Thank you all for the answers, now that you have all pointed it out it is easy to see the issues, I am mad at myself i failed to notice these mistakes.

EDIT: sorry to tag on another question but I just made all the fixes, and at first It was not working and after trying a bunch of different things, I ended up just trying to add using namespace std in, and that solved it, but I do not understand why that would fix my issues when i have been using std:: with everything.

For instance one of the issues I was getting was with line 6, the bool checkLegalUnits function after I added in the std::vector<string> legalUnits, it is saying that string is undefined.

Another error is with the iterator, the it = line, saying no operator = matches these operands, (although i think this could have something to do with the string issue, since if that is undefined the the iterator couldnt = it)

A lot of other issues are just after and end curly brace and just says expected a declaration.

Does anyone know why adding namespace std would solve these issues when i am adding std:: before streams, strings and whatever else needs it
Last edited on
seeplus (6457)
Show your latest code.
jlb (4973)
For instance one of the issues I was getting was with line 6, the bool checkLegalUnits function after I added in the std::vector<string> legalUnits, it is saying that string is undefined.


Did you properly scope that std class (std::string)?

Another error is with the iterator, the it = line, saying no operator = matches these operands, (although i think this could have something to do with the string issue, since if that is undefined the the iterator couldnt = it)

Have you considered using a Ranged Based Loop instead of the "iterator" version (for(auto& itr : YourVectorInstanceName)).

Also make sure you're working on the errors in order (top to bottom), one error may cause multiple messages. Compile after making changes (always compile early and often).



George P (5590)
Another error is with the iterator, the it = line, saying no operator = matches these operands, (although i think this could have something to do with the string issue, since if that is undefined the the iterator couldnt = it)


auto works wonders when dealing with iterators, the compiler determines the exact type of iterator needed so you don't have to guess:
for (auto it = legalUnits.begin(); it != legalUnits.end(); it++)

Range-based for loops, jlb showed an example, work well also.
DonnaPin (154)
Sorry should have added the code at the time. The code I have pasted below is not the exact same as the one I was having problems as I had to add more to it, but the errors I was having such as the string, is the same.

Some of the newer vectors etc that I have added in since my last post is no longer using the scope resolution operator and that is just because i am now using the std namespace, so that wasnt causing the issue at the time.

As for auto I have actually just started to look into that and seeing how it works, looks a lot easier than the iterator version. 


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
 

  // Example program
#include <iostream>
#include <string>
#include <vector>

using namespace std;

bool checkLegalUnits(std::string unit, std::vector<string> legalUnits)
{

    bool isLegal = false;
    std::vector<std::string>::iterator it;

    for (it = legalUnits.begin(); it != legalUnits.end(); it++)
    {
        if (*it == unit)
        {
            isLegal = true;
        }
    }

    return isLegal;
}

int main()
{
    double num;
    double smallest, largest;
    bool firstInput = true;
    std::string unit = " ";
    std::vector<std::string>legalUnits{ "cm", "m", "in", "ft" };
    vector<int>finalValues;
    int test;




    while (std::cin >> num)
    {
        std::cout << "Enter a unit of measurement \n";
        std::cin >> unit;

        if (checkLegalUnits(unit, legalUnits))
        {
            if (unit == "m")
            {
                num *= 100;
            }
            else if (unit == "in")
            {
                num *= 2.54;
            }
            else if (unit == "ft")
            {
                num = 12 * num * 2.54;
            }

            if (firstInput)
            {
                smallest = num;
                largest = num;
                firstInput = false;
            }



            if (num < smallest)
            {
                smallest = num;
                std::cout << smallest << unit << "\n";
            }
            else if (num > largest)
            {
                largest = num;
                std::cout << largest << unit << "\n";
            }




            std::cout << "Smallest : " << smallest << "cm" << " Largest :" << largest << "cm" << "\n";

        }
        //convert to meters to push to vector  
        test = num /= 100;
        finalValues.push_back(test);

    }

    vector<int>::iterator it;
    std::cout << "All values in meters : ";

    for (it = finalValues.begin(); it != finalValues.end(); it++)
    {
        std::cout << "\n" << *it;
    }

    return 0;
}
jlb (4973)
Line 9 was probably the reason your previous code was causing problems when not using the using clause, you forgot to properly scope the std::string for the parameter. By the way I really recommend you stick with scoping the standard classes and functions as required. Using the namespace clause can cause difficult to find problems at times.

You probably should also be passing those parameters by reference, instead of by value. It also looks like you could put that "unit" vector into the function, possibly as a constant instance, since that function is the only place that vector is used.

Lastly for now, you really should start getting used to the idea of declaring variables close to first use instead of a big glob at the beginning of some scope.



George P (5590)
As for auto I have actually just started to look into that and seeing how it works, looks a lot easier than the iterator version.

A suggestion: if you are simply 'walking through' your container without modifying any data you could/should use constant iterators, C++14 or later:
for (auto it = finalValues.cbegin(); it != finalValues.cend(); it++)

One nice feature about using iterators is you can do a reverse iteration, C++14 or later:
for (auto it = finalValues.crbegin(); it != finalValues.crend(); it++)
https://en.cppreference.com/w/cpp/iterator/rbegin
seeplus (6457)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
bool checkLegalUnits(std::string unit, std::vector<string> legalUnits)
{

    bool isLegal = false;
    std::vector<std::string>::iterator it;

    for (it = legalUnits.begin(); it != legalUnits.end(); it++)
    {
        if (*it == unit)
        {
            isLegal = true;
        }
    }

    return isLegal;
}


1) it can be defined using auto to save having to know its exact type and defined within the for loop

2) Once the unit is found, you continue searching even though the rest of the searching will be false.

If you want to use iterators, then

1
2
3
4
5
6
7
8
 
bool checkLegalUnits(const std::string& unit, const std::vector<std::string>& legalUnits) // Note const ref
{
	for (auto it = legalUnits.cbegin(); it != legalUnits.cend(); ++it) // Note auto, cbegin/cend and ++it
		if (*it == unit)
			return true;	// If found, just return true

	return false;		// Not found so return false
}


You don't need to use iterator here, you could just use a range-for

1
2
3
4
5
6
7
8
 
bool checkLegalUnits(const std::string& unit, const std::vector<std::string>& legalUnits) // Note const ref
{
	for (const auto& l : legalUnits)	// Range for const ref
		if (l == unit)
			return true;	// Found so return true

	return false;		// Not found so return false
}


However, this can be done simply using std::find

1
2
3
4
 
bool checkLegalUnits(const std::string& unit, const std::vector<std::string>& legalUnits) // Note const ref
{
	return std::find(legalUnits.cbegin(), legalUnits.cend(), unit) != legalUnits.cend();
}


1
2
3
4
5
6
 
if (firstInput)
            {
                smallest = num;
                largest = num;
                firstInput = false;
            }


You don't need this. When you initialize, set smallest to the maximum for the type and largest to the smallest for the type

 
 
double smallest {std::numeric_limits<double>::max()}, largest {std::numeric_limits<double>::min()};


Topic archived. No new replies allowed.