cv qualifier error

[fizanimtiaz93]

I'm getting the following error in my ADT bag code:

In file included from VectorBag.h:36:0,
                 from proj2.cpp:28:
VectorBag.cpp: In instantiation of ‘bool VectorBag<ItemType>::remove(const ItemType&) [with ItemType = std::basic_string<char>]’:
proj2.cpp:117:40:   required from ‘void bagTester(VectorBag<ItemType>&) [with ItemType = std::basic_string<char>]’
proj2.cpp:44:17:   required from here
VectorBag.cpp:27:13: error: passing ‘const std::basic_string<char>’ as ‘this’ argument of ‘std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’ discards qualifiers [-fpermissive]
     anEntry = items[items.size()];
             ^

The problem is that I'm giving a value to a cv-qualifier, so I either have to make a copy of it, or pass it by value. But I'm not allowed to change anything in the code, so I'm kinda having some real trouble with the situation. Could anyone help me out? I've been at this for weeks. I didn't post the whole code to keep it easy to read, but if you need it, let me know and I'll post it up. But here is the snippet with the source of the error:

template<class ItemType>
bool VectorBag<ItemType>::remove(const ItemType& anEntry) {
    ItemType temp = anEntry;
    anEntry = items[items.size()];
    items[items.size()]  = temp;
    items.pop_back();
    return true;
}

[AbstractionAnon]

Line 4: anEntry is described as const. Therefore you can't assign anything to it.

Line 4: You're subscripting out of bounds. if items has 10 entries, those items are 0-9.

Line 5: You have no guarantee that size() items are allocated. If you're trying to add an item here, you need to do items.push_back(temp)

Line 6: What's the point of popping the item you just added?

Is the purpose of remove to remove a specific entry (anEntry)? If so, don't you need to search the vector for the matching entry?

[fizanimtiaz93]

This function is not designed to add anything to the vector. Its supposed to swap the last item in items, which items.size() points to, with anEntry, and once anEntry becomes the final entry in the vector, items.pop_back() removes it.

> Line 4: anEntry is described as const. Therefore you can't assign anything to it.

Right. So how would I go about dealing with that?

> Line 4: You're subscripting out of bounds. if items has 10 entries, those items are 0-9.

I'm confused. Are you saying that items.size() is items 0 - 9 ?

> Is the purpose of remove to remove a specific entry (anEntry)? If so, don't you need to search the vector for the matching entry?

You're right about, I'll go ahead and fix up the code. Meantime, how would I pass by value here or make a copy of the cv-qualifier variable?

[AbstractionAnon]

the last item in items, which items.size() points to

No. items[items.size()] is out of bounds.
As I said before, if size is 10, then the valid entries are 0 - 9. items[10] is not valid.

I'm not understanding why you're trying to swap something into anEntry. If you can't change the function declaration, then it would seem that was not the intent.

Are you saying that items.size() is items 0 - 9 ?

Yes. Arrays and vectors in C++ are numbered from 0. Therefore, valid entries are 0 - 9.

how would I pass by value here

I thought you said you can't change the declaration, which is what you would have to do to pass by value. In any case, I'm not sure that's what you want to do.

or make a copy of the cv-qualifier variable?

temp is in fact a copy of anEntry, but I'm guessing that you don't need that.

Its supposed to swap the last item in items, which items.size() points to, with anEntry, and once anEntry becomes the final entry in the vector, items.pop_back() removes it.

I'm sorry, but that makes no sense to me given that anEntry is const. What is the point of making anEntry the final entry in the vector, only to pop it?

It makes a lot more sense to me that your remove function would search the vector to find an entry that matches anEntry. A simple for loop would work for this assuming ItemType has an equality operator. If a matching entry is not found, return false. If a matching entry is found, there use items.erase() to get rid of the entry you found.

[fizanimtiaz93]

oh, I get what you're saying about the items.size() business now, thanks for clearing all that up. I can't believe I missed something so obvious.

anyways, here's what I got when I changed it up to find the index of anEntry and erase it:

template<class ItemType>
bool VectorBag<ItemType>::remove(const ItemType& anEntry) {
    int i = 0;
    while (items[i] != anEntry) i++;
    items.erase(items[i]);
    return true;
}

and the error message now:

In file included from VectorBag.h:36:0,
                 from proj2.cpp:28:
VectorBag.cpp: In instantiation of ‘bool VectorBag<ItemType>::remove(const ItemType&) [with ItemType = std::basic_string<char>]’:
proj2.cpp:117:40:   required from ‘void bagTester(VectorBag<ItemType>&) [with ItemType = std::basic_string<char>]’
proj2.cpp:44:17:   required from here
VectorBag.cpp:28:5: error: no matching function for call to ‘std::vector<std::basic_string<char>, std::allocator<std::basic_string<char> > >::erase(std::basic_string<char>&)’
     items.erase(items[i]);
     ^

I don't understand why it's saying that, when I did #include <vector> at the top.

[AbstractionAnon]

Don't you just love error messages involving templates? :)

What that cryptic message message is trying to tell you is that std::vector does not have an erase function that takes an item by value, which is what you specified at line 5. erase requires an iterator, not a value type.

There is also a problem with your while loop. What happens if anEntry is not in the vector? You have no condition to exit out of the loop when you reach the end of the vector. This was why I suggested a for loop.

Consider the following:

//  Assumes anEntry occurs only once in the vector.
//  If multiple entries exist, will delete the first occurrance
//  Also assumes ItemType has a comparison operator 
template<class ItemType>
bool VectorBag<ItemType>::remove(const ItemType& anEntry) 
{  ItemType::const_iterator  citer;  
    for (citer=items.begin(); citer!=items.end(); citer++)
        if (*citer == anEntry) 
        {  items.erase (citer); 
            return true;  // requested entry was found and deleted          
        }
    return false;  // anEntry not found
} 

[fizanimtiaz93]

I tried putting that into my code, and what do you know...more error messages lol. I tried changing it up to make it work, but my attempts didn't work either so I just reverted it back to what you wrote. Here's what I got:

VectorBag.cpp: In member function ‘bool VectorBag<ItemType>::remove(const ItemType&)’:
VectorBag.cpp:26:5: error: need ‘typename’ before ‘ItemType:: const_iterator’ because ‘ItemType’ is a dependent scope
     ItemType::const_iterator citer;
     ^
VectorBag.cpp:26:30: error: expected ‘;’ before ‘citer’
     ItemType::const_iterator citer;
                              ^
VectorBag.cpp:27:10: error: ‘citer’ was not declared in this scope
     for (citer=items.begin(); citer!=items.end(); citer++)
          ^
VectorBag.cpp: In instantiation of ‘bool VectorBag<ItemType>::remove(const ItemType&) [with ItemType = std::basic_string<char>]’:
proj2.cpp:117:40:   required from ‘void bagTester(VectorBag<ItemType>&) [with ItemType = std::basic_string<char>]’
proj2.cpp:44:17:   required from here
VectorBag.cpp:26:5: error: dependent-name ‘ItemType:: const_iterator’ is parsed as a non-type, but instantiation yields a type
     ItemType::const_iterator citer;
     ^

[AbstractionAnon]

Since you didn't post vectorbag.h, I was uncertain of the type of items, nor could I compile the code I posted to check for errors.

Line 6 should be an iterator of whatever type items is.
Assuming items is: vector<ItemType> items; then line 6 should be:

  vector<ItemType>::const_iterator  citer;  

If items is some type other than vector<ItemType> then you need to adjust the iterator accordingly.

Sorry for the confusion.

[fizanimtiaz93]

I'm still getting errors, I don't know why.

Here's VectorBag.h, to make it easier for you:

#ifndef VECTORBAG_H
#define VECTORBAG_H
#include "BagInterface.h"
    
   template<class ItemType>
   class VectorBag: public BagInterface<ItemType>
   {
     public:
       int getCurrentSize() const;
       bool isEmpty() const;
       bool add(const ItemType& newEntry);
       bool remove(const ItemType& anEntry);
       void clear();
       bool contains(const ItemType& anEntry) const;
       int getFrequencyOf(const ItemType& anEntry) const;
       vector<ItemType> toVector() const;
   
       VectorBag<ItemType> operator+(VectorBag<ItemType> anotherBag);
    
       VectorBag<ItemType> operator*(VectorBag<ItemType> anotherBag);
    
       VectorBag<ItemType> operator-(VectorBag<ItemType> anotherBag);
    
    private:
           vector<ItemType> items;              // where we store
						  // the bag
           // Returns either the index of the element in the array
           // items that
           // contains the given target or -1, if the array does not
           // contain 
           // the target.
           int getIndexOf(const ItemType& target) const;   
    
    }; // end VectorBag

#include "VectorBag.cpp"
#endif 

and BagInterface.h:

#ifndef BAGINTERFACE_H
#define BAGINTERFACE_H

#include <vector>
using namespace std;

template<class ItemType>
class BagInterface
{
public:
    virtual int getCurrentSize() const = 0;
   
    virtual bool isEmpty() const = 0;
   
    virtual bool add(const ItemType& newEntry) = 0;
   
    virtual bool remove(const ItemType& anEntry) = 0;
   
    virtual void clear() = 0;
   
    virtual int getFrequencyOf(const ItemType& anEntry) const = 0;
   
    virtual bool contains(const ItemType& anEntry) const = 0;

    virtual vector<ItemType> toVector() const = 0;

}; // end BagInterface
#endif 

[AbstractionAnon]

vectorbag.h line 38: Never include a .cpp file in a header. This will cause linker errors.

Can you please be more specific about the errors you're getting and where they are occurring.

It would be helpful if you could post vetorbag.cpp.

[fizanimtiaz93]

The files are all given to me by my professor, I just have to implement it in VectorBag.cpp.
So here it is (and by the way, you can ignore newOpsTester() for now):

#include <algorithm>
#include <iostream>
#include <string>
#include "VectorBag.h"

using namespace std;

template<class ItemType> void displayBag(VectorBag<ItemType>& bag);
template<class ItemType> void bagTester(VectorBag<ItemType>& bag);
// void newOpsTester();

/**
 * The usual <code>main()</code> function.
 */
int main()
{
   VectorBag<string> bag;
   cout << "Testing the Vector-Based Bag:" << endl;
   cout << "The initial bag is empty." << endl;
   bagTester(bag);
   // newOpsTester();
   cout << "All done!" << endl;
   
   return 0;
}

/**
 * Display the contents of a <code>VectorBag</code>, which can hold
 * any kind of element.  The elements are displayed in sorted order.
 * (This means that the <code>ItemType</code> must be one for which
 * the&nbsp;"&lt;" operation is defined.)
 * @param bag The bag to be displayed.
 */
template<class ItemType>
void displayBag(VectorBag<ItemType>& bag)
{
   cout << "The bag contains " << bag.getCurrentSize()
        << " items:" << endl;
   vector<ItemType> bagItems = bag.toVector();
   
   int numberOfEntries = static_cast<int>(bagItems.size());
   sort(bagItems.begin(), bagItems.end());

   for (int i = 0; i < numberOfEntries; i++) 
      cout << bagItems[i] << " ";
   cout << endl << endl;
}  // end displayBag

/**
 * This is an adaption of the <code>bagTester</code> function in 
 * Carrano-Henry <code>main.cpp</code>, which tests the
 * <code>ArrayBag</class>.  The changes are as follows:
 * <ol>
 * <li> Whereas the original version assumed that
 * the <code>ArrayBag</code> contained <code>string</code>, our
 * version allows the <code>VectorBag</code> to contain any kind of
 * item.</li>
 * <li> In addition to testing the original operations that are
 * implemented from <code>BagInterface</code>, we test the operations
 * of union, intersection, and difference.</li>
 * </ol>
 * @param bag The bag to be tested.
 */
template<class ItemType> void bagTester(VectorBag<ItemType>& bag)
{
   cout << "isEmpty: returns " << bag.isEmpty() 
        << "; should be 1 (true)" << endl;
   displayBag(bag);

   string items[] = {"one", "two", "three", "four", "five", "one"};
   int itemsSize = sizeof(items)/sizeof(char*);
   cout << "Adding " << itemsSize << " items to the bag: " << endl;
   for (unsigned int i = 0; i < itemsSize; i++)
      bag.add(items[i]);
   
   displayBag(bag);
   
   cout << "isEmpty: returns " << bag.isEmpty() 
        << "; should be 0 (false)" << endl;
	
   cout << "getCurrentSize: returns " << bag.getCurrentSize() 
        << "; should be " << itemsSize << endl;
   
   cout << "Try to add another entry: add(\"extra\") returns " 
        << bag.add("extra") << endl;      
   
   cout << "contains(\"three\"): returns " << bag.contains("three")
        << "; should be 1 (true)" << endl;
   cout << "contains(\"ten\"): returns " << bag.contains("ten")
        << "; should be 0 (false)" << endl;
   cout << "getFrequencyOf(\"one\"): returns "
        << bag.getFrequencyOf("one") << " should be 2" << endl;
   cout << "remove(\"one\"): returns " << bag.remove("one")
        << "; should be 1 (true)" << endl;
   cout << "getFrequencyOf(\"one\"): returns "
        << bag.getFrequencyOf("one") << " should be 1" << endl;
   cout << "remove(\"one\"): returns " << bag.remove("one")
        << "; should be 1 (true)" << endl;
   cout << "remove(\"one\"): returns " << bag.remove("one")
        << "; should be 0 (false)" << endl;
   cout << endl;
   
   displayBag(bag);
   
   cout << "After clearing the bag, ";
   bag.clear();
   
   cout << "isEmpty: returns " << bag.isEmpty()
        << "; should be 1 (true)" << endl;
}  // end bagTester

// void newOpsTester()
// {
//    cout << "\nLet's test the new bag operations.\n\n";
//    int firstItems[] = {2, 4, 6, 6, 8, 10, 12, 15, 15};
//    VectorBag<int> firstBag;
//    int firstSize = sizeof(firstItems)/sizeof(int);
//    cout << "Adding " << firstSize << " items to first bag: " << endl;
//    for (int i = 0; i < firstSize; i++)
//       firstBag.add(firstItems[i]);
//    cout << "First bag: ";
//    displayBag(firstBag);

//    int secondItems[] = {3, 6, 9, 12, 12, 15, 15, 18};
//    VectorBag<int> secondBag;
//    int secondSize = sizeof(secondItems)/sizeof(int);
//    cout << "Adding " << secondSize << " items to second bag: " << endl;
//    for (int i = 0; i < secondSize; i++)
//       secondBag.add(secondItems[i]);
//    cout << "Second bag: ";
//    displayBag(secondBag);

//    VectorBag<int> unionBag = firstBag + secondBag;
//    cout << "firstBag union secondBag: \n";
//    displayBag(unionBag);

//    VectorBag<int> intersectBag = firstBag * secondBag;
//    cout << "firstBag intersect secondBag: \n";
//    displayBag(intersectBag);

//    VectorBag<int> diffBag = firstBag - secondBag;
//    cout << "firstBag minus secondBag: \n";
//    displayBag(diffBag);

//    diffBag = secondBag - firstBag;
//    cout << "secondBag minus firstBag: \n";
//    displayBag(diffBag);

// } 

You'll also notice a long line of error messages that are all "previously declared." That I'm trying to look up and fix by myself, but feel free to weigh in on what you think I should do.

[AbstractionAnon]

Compiled fine for me. I removed the #include "vectorbag.cpp" that was inside vectorbag.h.

I got one minor warning at vectorbag.cpp line 73. This should be an int instead of an unsigned int.

[fizanimtiaz93]

That's the problem! I'm not allowed to remove the #include VectorBag.cpp cause it's pre-written by my professor.

[MikeyBoy]

VevtorBag is a template class, which means you have to include the entire definition of the class and its methods in the translation unit that uses it. In other words, it's appropriate to include the .cpp file.

(That's actually an unusual way of doing it. The more normal way is to put all of the method definitions inside the class definition, in the header file. So you wouldn't have a VectorBag.cpp file at all - it would all be in VectorBag.h.

But your professor's way is also a valid way.)

If you're including VectorBag.cpp in VectorBag.h, you shouldn't also include VectorBag.h in VectorBag.cpp - although the inclusion guards you have in VectorBag.h should stop that causing problems.

Edit: Although it you're using VectorBag.cpp as an included file, it might help to put inclusion guards in that file as well.

Without knowing what your error messages are, that's all I can really say.

Is there any particular reason you're making it more difficult for yourself to get the help you need by not telling us what the error messages are?

[AbstractionAnon]

I'm a little lost on where you're at with this.

With regard to the #include "vectorbag.cpp" , I understand why your professor did that and that you can't change it. That said, I still believe it's a poor practice. .cpp files are meant to be compiled. Giving a file a .cpp suffix raises the possibility of confusion as to whether the file should be compiled or not. However, there are times when source code legtimately needs to be included within another file. Generated code is a good example of this. I prefer using a .inc suffix for these files making it clear the file is included in something else and as not meant to be compiled by itself.

[ne555]

> error: need ‘typename’ before ‘ItemType:: const_iterator’ because ‘ItemType’ is a dependent scope
do what the compiler is suggesting typename ItemType::const_iterator citer;


But the original code shouldn't be ItemType::const_iterator but actually vector<ItemType>::const_iterator citer;
and you need to add `typename' to that
typename vector<ItemType>::const_iterator citer;
http://www.parashift.com/c++-faq/nondependent-name-lookup-members.html


with c++11 I think that you could simply use auto citer = items.cbegin()