std::find method seems to have the effect of altering the arguments!

**FortranLevelC++** · 11-26-2018

Hello, and thank you for your precious time.

I have a question about the std::find method applied to std::vector. It seems to me that there is a danger that applying the std::find method to find a certain element in a vector might actually have the effect of altering the original vector in some way.

For the sake of completeness, I am using C++11 in the Ubuntu Linux environment with Code::Blocks, but I am sure that the same thing would happen in any other operating system and compiler.

I am pasting my own code at the end of the message, but before I do that, I am also pasting the reference to the std::find method:

http://www.cplusplus.com/

If I understood what that reference above says, that std::find above would somehow affect the original array (or vector in my case).

In my own function 'Vol_ForGroupOfSymbols( ... )' that I am pasting below, in the arguments of the function, if I make the second vector argument constant 'const' ( my function will search an element inside that vector argument by means of the std:find method), the compiler gives an error message and it says "error: no match for the operator ='. But if I relax the requirement that the vector argument that I am passing to my function should be const, then the compiler does not give an error message and the executable is produced without problem. Is there a way for me to make sure that the argument is not altered, at least after the function goes out of scope?

/*-------------------------------------------------------------------------*/
// Below, in my function ,Vol_ForGroupOfSymbols the issue is the second argument

Code:

const vector<string> & vec_GroupSymbolList

which the compiler does not allow me to pass it as a const, if I remove that 'const' in front of the argument

Code:

 vector<string> &  vec_GroupSymbolList

, then everything is fine, and the compiler does not complain. But if I try to make the argument constant as 'const' as in

Code:

const vector<string> &  vec_GroupSymbolList'

then the compiler gives that error message. What can I do to make sure that that vector argument that I am passing to my function is not in danger of being altered when the function is called? As a last resort I can certainly create an extra copy of the original vector and pass that extra copy to the function as argument, but this would take extra memory. Any suggestions will be appreciated.

Code:

/// vector<float> Vol_ForGroupOfSymbols( const  vector<RAStruct> & v1_RA_DataForRSSymbol,   vector<string> &  vec_GroupSymbolList,  const vector<vector<RAStruct > > & vec_LocalAllSymRecs  )


 vector<float> Vol_ForGroupOfSymbols( const  vector<RAStruct> & v1_RA_DataForRSSymbol, const vector<string> &  vec_GroupSymbolList,  const vector<vector<RAStruct > > & vec_LocalAllSymRecs  )
{
   vector<float> v1_VolForGroupOfSymbols;
   map<string, int> MapLocalDatesToCoordsForGuide;


   for(int i=0; i<v1_RA_DataForRSSymbol.size(); i++) {
      MapLocalDatesToCoordsForGuide[v1_RA_DataForRSSymbol[i].Date] = i ;
   }




   for(int i=0; i<v1_RA_DataForRSSymbol.size(); i++) {
      v1_VolForGroupOfSymbols.push_back(0); ; // Initialize to zero
   }




   std::vector<string>::iterator itrSymLoc;


   int FoundSymbolCounter  =0;
   int MissedSymbolCounter = 0;
   int iCoord;
   for (int i=0; i< vec_LocalAllSymRecs.size(); i++) { // here TotalNumSymbols is the num symbols in core RA file.
      int MissedDateCounter = 0;


      itrSymLoc = std::find(vec_GroupSymbolList.begin(), vec_GroupSymbolList.end(), vec_LocalAllSymRecs[i][0].Symbol   );


      if ( itrSymLoc != vec_GroupSymbolList.end()  ) {
         FoundSymbolCounter++;
         for (int j =0; j<vec_LocalAllSymRecs[i].size(); j++) {// Note that index j starts from j=1 instead of j=0
            if ((MapLocalDatesToCoordsForGuide.count( vec_LocalAllSymRecs[i][j].Date  ) >0) &&
                  ((iCoord ==  Map_DateToCoordsRefSym[ vec_LocalAllSymRecs[i][j].Date ]) > 0)) {


               v1_VolForGroupOfSymbols[iCoord  ] +=  vec_LocalAllSymRecs[i][j].Volume ;




            } else {
               MissedDateCounter++;
            }
         }


      } else {
         MissedSymbolCounter++;


      }


   }


   return v1_VolForGroupOfSymbols;
}

**john.c** · 11-26-2018

Of course find doesn't alter the container. That's ridiculous.
Post a small, complete program that demonstrates the "problem" (and please format it properly).
BTW, cplusplus.com is not a very good reference. The pros use cppreference.com

Guest · 11-26-2018

If I understood what that reference above says, that std::find above would somehow affect the original array (or vector in my case).

That's not the case, std::find only modifies the iterators passed, but not their contents. You can see this by passing const_iterators to the function, i.e.:

Code:

itrSymLoc = std::find(vec_GroupSymbolList.cbegin(), vec_GroupSymbolList.cend(), vec_LocalAllSymRecs[i][0].Symbol);

I suspect you're seeing some other effect that's causing the error message and confusing you. For instance, if Symbol is not a basic type, you'll need to overload the comparison operator (or use std::find_if with a predicate) to test two Symbols for equivalence.

**FortranLevelC++** · 11-26-2018

John C,
Thank you for the comments. I also think that the std::find doesn't alter the container. But passing that argument as const to my function above is giving an error message, apparently because this argument is searched by the std::find function above.

The only problem is the error message that the compiler gives when the second argument of the function is 'const" as illustrated above.
That argument in question is the vector<string> & vec_GroupSymbolList

The function that I pasted above is complete, the full program is 20,000 lines long, but I can certainly write a separate demo program to figure this out and paste it.
In any case, the compiler error message is due to the 'const' in front of that argument.

The error message that the compiler gives when that argument is const, is: "error: no match for the operator='.

When I remove the 'const' in front of the vector argument (as commented above in my program segment),then the compiler error goes away.

**FortranLevelC++** · 11-26-2018

Adrian,
Thank you for the comments. Yes, it is the iterator that is being altered, but not the contents!!!
But you raised an important issue here. Yes, the type "Symbol" is not simply a string, but instead, it is an array of chars. The compiler is smart enough to make the comparison between an array of chars and a string, but maybe it is causing the error message when I make the argument const.

Specifically, the vector element vec_LocalAllSymRecs[i][j] is a struct of the form:

Code:

struct RAStruct { //
   char Symbol[12]; // 
   char Date[12];
   float Open;
   float High;
   float Low;
   float Close;
   int Volume;
};

Maybe this is why the compiler does not allow me to make that argument const?

But without that const in front of the argument, the comparison between the array of chars and the string-valued vector elements, is being made without problem

**FortranLevelC++** · 11-26-2018

Originally Posted by Guest

That's not the case, std::find only modifies the iterators passed, but not their contents. You can see this by passing const_iterators to the function, i.e.:

Code:

itrSymLoc = std::find(vec_GroupSymbolList.cbegin(), vec_GroupSymbolList.cend(), vec_LocalAllSymRecs[i][0].Symbol);

I suspect you're seeing some other effect that's causing the error message and confusing you. For instance, if Symbol is not a basic type, you'll need to overload the comparison operator (or use std::find_if with a predicate) to test two Symbols for equivalence.

Thanks again, but in this case how can I overload the comparison operator?

Here, vec_LocalAllSymRecs[i][0] is of the form:

Code:

struct RAStruct { //
   char Symbol[12]; // 
   char Date[12];
   float Open;
   float High;
   float Low;
   float Close;
   int Volume;
};

while the elements of the vector vec_GroupSymbolList are strings.
But once again the code works fine when there is no const in front of vec_GroupSymbolList that is the second argument of my outer function.

**FortranLevelC++** · 11-26-2018

Originally Posted by Guest

That's not the case, std::find only modifies the iterators passed, but not their contents. You can see this by passing const_iterators to the function, i.e.:

Code:

itrSymLoc = std::find(vec_GroupSymbolList.cbegin(), vec_GroupSymbolList.cend(), vec_LocalAllSymRecs[i][0].Symbol);

I suspect you're seeing some other effect that's causing the error message and confusing you. For instance, if Symbol is not a basic type, you'll need to overload the comparison operator (or use std::find_if with a predicate) to test two Symbols for equivalence.

I tried to introduce a temporary intermediate variable to make sure that the variable that is being searched is a string, but it did not fix the problem.

Code:

string tempS = (string)vec_LocalAllSymRecs[i][0].Symbol;
itrSymLoc = std::find(vec_GroupSymbolList.begin(), vec_GroupSymbolList.end(), tempS   );

is still giving the same error when the argument vec_GroupSymbolList is passed as a constant to the function.

**laserlight** · 11-26-2018

Originally Posted by FortranLevelC++

Maybe this is why the compiler does not allow me to make that argument const?

No, your problem almost certainly has to do with mismatching iterator types, as Guest alluded to in post #3. You should declare variables near first use, preferably at the point where they can be sensibly initialised. So, what I would have done instead of:

Code:

   std::vector<string>::iterator itrSymLoc;

   // ...

   for (int i=0; i< vec_LocalAllSymRecs.size(); i++) { // here TotalNumSymbols is the num symbols in core RA file.

      // ...

      itrSymLoc = std::find(vec_GroupSymbolList.begin(), vec_GroupSymbolList.end(), vec_LocalAllSymRecs[i][0].Symbol   );

Is to write something like this:

Code:

   for (int i=0; i< vec_LocalAllSymRecs.size(); i++) { // here TotalNumSymbols is the num symbols in core RA file.

      // ...

      auto itrSymLoc = std::find(vec_GroupSymbolList.begin(), vec_GroupSymbolList.end(), vec_LocalAllSymRecs[i][0].Symbol );

std::find can return an iterator to non-const because that's one possible usage: find an item and then modify it. However, you declared itrSymLoc to be an iterator to non-const, yet you declared vec_GroupSymbolList to be a const reference: clearly something isn't right. By using auto, I eliminated this mismatch in types.

EDIT:
When you write comments, you must maintain them. You left this comment that makes no sense at all:

Code:

// here TotalNumSymbols is the num symbols in core RA file.

There's no identifier TotalNumSymbols, so what's this about? Thankfully, it is easy to see that this is probably just a harmless outdated comment.

But then you wrote:

Code:

for (int j =0; j<vec_LocalAllSymRecs[i].size(); j++) {// Note that index j starts from j=1 instead of j=0

Okay, so the index j starts from j=1... hold on, in the code it starts from j=0, so is this a bug with the code or is this a bug with the comment, or both?

Note that you don't seem to assign to iCoord despite using its value.

**FortranLevelC++** · 11-26-2018

Originally Posted by laserlight

No, your problem almost certainly has to do with mismatching iterator types, as Guest alluded to in post #3. You should declare variables near first use, preferably at the point where they can be sensibly initialised. So, what I would have done instead of:

Code:

   std::vector<string>::iterator itrSymLoc;

   // ...

   for (int i=0; i< vec_LocalAllSymRecs.size(); i++) { // here TotalNumSymbols is the num symbols in core RA file.

      // ...

      itrSymLoc = std::find(vec_GroupSymbolList.begin(), vec_GroupSymbolList.end(), vec_LocalAllSymRecs[i][0].Symbol   );

Is to write something like this:

Code:

   for (int i=0; i< vec_LocalAllSymRecs.size(); i++) { // here TotalNumSymbols is the num symbols in core RA file.

      // ...

      auto itrSymLoc = std::find(vec_GroupSymbolList.begin(), vec_GroupSymbolList.end(), vec_LocalAllSymRecs[i][0].Symbol );

std::find can return an iterator to non-const because that's one possible usage: find an item and then modify it. However, you declared itrSymLoc to be an iterator to non-const, yet you declared vec_GroupSymbolList to be a const reference: clearly something isn't right. By using auto, I eliminated this mismatch in types.

EDIT:
When you write comments, you must maintain them. You left this comment that makes no sense at all:

Code:

// here TotalNumSymbols is the num symbols in core RA file.

There's no identifier TotalNumSymbols, so what's this about? Thankfully, it is easy to see that this is probably just a harmless outdated comment.

But then you wrote:

Code:

for (int j =0; j<vec_LocalAllSymRecs[i].size(); j++) {// Note that index j starts from j=1 instead of j=0

Okay, so the index j starts from j=1... hold on, in the code it starts from j=0, so is this a bug with the code or is this a bug with the comment, or both?

Note that you don't seem to assign to iCoord despite using its value.

Laserlight,

Thank you for solving the problem so quickly! A few years ago you also solved many similar mysteries for me!

As you suggested I used auto to assign the right type of iterator, this solved the problem.

I incorrectly imagined that making the argument 'const' does not change its type and that it just prevents the function from altering the argument,
but apparently, behind the scenes, it is the type of the argument that is fundamentally changed.

Without your help I would not have solved the mystery because I was looking in the wrong direction.

I do apologize for the distracting comments that I failed to remove.

As for the indexing issues , the function that I was writing is still incomplete, I was only testing the syntax, not the math part.
The j = 1 was the old generation of code that was correct, the new generation has j = 0 that is also correct, but the comments stayed.
I do apologize for the confusion, but the question has been resolved with your help!

But I don't know how to thank you for your dedication and help. I have been using many C++ books, especially Alex Allain's text, but the iterator issue was very subtle and the misconception about what const does to an argument was serious. That being said, this C++ code already has over 20,000 lines.

**laserlight** · 11-26-2018

You're welcome. I really do encourage you to look into the "declare variables near first use" thing more closely. Here's another case:

Code:

   vector<float> v1_VolForGroupOfSymbols;
   map<string, int> MapLocalDatesToCoordsForGuide;


   for(int i=0; i<v1_RA_DataForRSSymbol.size(); i++) {
      MapLocalDatesToCoordsForGuide[v1_RA_DataForRSSymbol[i].Date] = i ;
   }




   for(int i=0; i<v1_RA_DataForRSSymbol.size(); i++) {
      v1_VolForGroupOfSymbols.push_back(0); ; // Initialize to zero
   }

I would ask: is there a way to initialise the variables with what their initial values should be? It seems unlikely for MapLocalDatesToCoordsForGuide, but it can be done for v1_VolForGroupOfSymbols:

Code:

   vector<float> v1_VolForGroupOfSymbols(v1_RA_DataForRSSymbol.size(), 0);

   map<string, int> MapLocalDatesToCoordsForGuide;
   for (int i=0; i<v1_RA_DataForRSSymbol.size(); i++) {
      MapLocalDatesToCoordsForGuide[v1_RA_DataForRSSymbol[i].Date] = i;
   }

Notice too that I have grouped the declaration of MapLocalDatesToCoordsForGuide with the code that gives it its initial value so as to reduce the chance that more code will be added in between, thereby obscuring the declaration and assignment of initial values.

Another thing to consider: when you loop over vec_LocalAllSymRecs, do you really need the index variable named i? If not, consider using a range-based for loop. This way, in the loop body you consider the current element through the context of a named reference, rather than considering the current element through the more cumbersome vec_LocalAllSymRecs[i].

**FortranLevelC++** · 11-26-2018

=laserlight;1281609]You'rewelcome. I really do encourage you to look into the "declarevariables near first use" thing more closely. Here's anothercase:

I would ask: is there a way to initialise the variables with what theirinitial values should be? It seems unlikely forMapLocalDatesToCoordsForGuide, but it can be done forv1_VolForGroupOfSymbols:

Notice too that I have grouped the declaration ofMapLocalDatesToCoordsForGuide with the code that gives it its initialvalue so as to reduce the chance that more code will be added inbetween, thereby obscuring the declaration and assignment of initialvalues.

Another thing to consider: when you loop over vec_LocalAllSymRecs, do youreally need the index variable named i? If not, consider using arange-based for loop. This way, in the loop body you consider thecurrent element through the context of a named reference, rather thanconsidering the current element through the more cumbersomevec_LocalAllSymRecs[i].

Thanks to your suggestions, Here is the new code with your ideas about compact initialization,logical grouping, and range-based for loops.
It is definitely more effective:

Code:

vector<float> Vol_ForGroupOfSymbols( const  vector<RAStruct> & v1_RA_DataForRSSymbol, const vector<string> &  v1_GroupSymbolList,  const vector<vector<RAStruct > > & v2_LocalAllSymRecs  )
{
   vector<float> v1_VolForGroupOfSymbols(v1_RA_DataForRSSymbol.size(), 0); /// Note how we initialized vector to 0 while we set its dimension to v1_RA_DataForRSSymbol.size()


   map<string, int> MapLocalDatesToCoordsForGuide;
   for(int i=0; i<v1_RA_DataForRSSymbol.size(); i++) {
      MapLocalDatesToCoordsForGuide[v1_RA_DataForRSSymbol[i].Date] = i ;
   }


   /// Used auto below instead of risking defining incorrect iterator itrSymLoc directly .
  ///  std::vector<string>::const_iterator itrSymLoc; /// const_iterator corresponds to the const vector<string> argument of function.
   int FoundSymbolCounter  =0;
   int MissedSymbolCounter;




   for (auto group : v2_LocalAllSymRecs ) {
      int MissedDateCounter = 0;
      auto itrSymLoc = std::find(v1_GroupSymbolList.begin(), v1_GroupSymbolList.end(), group[0].Symbol);
      if ( itrSymLoc != v1_GroupSymbolList.end()  ) {
         FoundSymbolCounter++;
         for ( auto record : group ) {
            if ( MapLocalDatesToCoordsForGuide.count( record.Date  ) >0 ) {
               int iCoord =  Map_DateToCoordsRefSym[ record.Date ];
               v1_VolForGroupOfSymbols[iCoord  ] +=  record.Volume ;
            } else {
               MissedDateCounter++;
            }
         }


      } else {
         MissedSymbolCounter++;


      }


   }


   return v1_VolForGroupOfSymbols;
}

However,quite often I need to access individual elements surgically, and I am addicted to using indices due to numerical programming with nested loops.
Fort his function your range-based for loops work very well..

**laserlight** · 11-26-2018

I'm not sure what's up with the formatting you used, but I'd suggest:

Code:

vector<float> Vol_ForGroupOfSymbols(const vector<RAStruct>& v1_RA_DataForRSSymbol,
                                    const vector<string>& v1_GroupSymbolList,
                                    const vector<vector<RAStruct>>& v2_LocalAllSymRecs)
{
    vector<float> v1_VolForGroupOfSymbols(v1_RA_DataForRSSymbol.size(), 0);

    map<string, int> MapLocalDatesToCoordsForGuide;
    for (int i = 0; i < v1_RA_DataForRSSymbol.size(); ++i) {
        MapLocalDatesToCoordsForGuide[v1_RA_DataForRSSymbol[i].Date] = i;
    }

    int FoundSymbolCounter = 0;
    int MissedSymbolCounter = 0;

    for (const auto& group : v2_LocalAllSymRecs) {
        int MissedDateCounter = 0;
        auto itrSymLoc = std::find(v1_GroupSymbolList.begin(),
                                   v1_GroupSymbolList.end(),
                                   group[0].Symbol);
        if (itrSymLoc != v1_GroupSymbolList.end()) {
            ++FoundSymbolCounter;

            for (const auto& record : group) {
                auto iter = MapLocalDatesToCoordsForGuide.find(record.Date);
                if (iter != MapLocalDatesToCoordsForGuide.end()) {
                    v1_VolForGroupOfSymbols[iter->second] += record.Volume;
                } else {
                    ++MissedDateCounter;
                }
            }
        } else {
            ++MissedSymbolCounter;
        }
    }

    return v1_VolForGroupOfSymbols;
}

**FortranLevelC++** · 11-26-2018

Thanks again for the crucial comment because many of my functions have nearly a dozen arguments.

At least when I am defining a function it is a good idea to format the arguments like you did above, but this function had only 3 arguments and it was visible in the browser,
but probably some browsers don't show the lines very well.

But because I will soon end up with hundreds of functions in the main program itself, I try to save space and put at least 3 or 4 arguments per line when a function is called.
Perhaps, another option is to create a struct to pack all the arguments.

Unfortunately, it seems that when functions have different kinds of arguments, it is not possible or practical to use a for loop to execute an array of functions like in the other question,
unless it is easy to overload the definition of the array or map of functions, but this seems too dangerous.

**laserlight** · 11-26-2018

Originally Posted by FortranLevelC++

But because I will soon end up with hundreds of functions in the main program itself, I try to save space and put at least 3 or 4 arguments per line when a function is called.
Perhaps, another option is to create a struct to pack all the arguments.

I wouldn't worry about vertical space so much. You can always split the code into multiple source files, and generally as your functions should do one thing and do it well, they shouldn't be so long as to require much scrolling, if any. Therefore, having many functions don't really matter so much... unless of course you have global state such that you need to keep peeking into functions that are called.

Originally Posted by FortranLevelC++

Unfortunately, it seems that when functions have different kinds of arguments, it is not possible or practical to use a for loop to execute an array of functions like in the other question,
unless it is easy to overload the definition of the array or map of functions, but this seems too dangerous.

While I'm not convinced that your idea of working directly with function names read from a file is a good idea (it feels like you're tightly coupled to your implementation), given what you're trying to do, you could define a function object base class and overload operator() for the different parameter lists you have in mind. If you make these overloaded functions pure virtual, you can then derive from this base class in the knowledge that in order to instantiate the function objects, you need to override each of these overloaded pure virtual member functions, so if you miss one you'll get an error when populating your container of function objects (which would probably then be a std::map<std::string, std::unique_ptr<AverageFunction>>, since you need to store pointers to the AverageFunction base class in order to use polymorphism).

**FortranLevelC++** · 11-26-2018

Thanks again for the suggestions about overloading the () operators. I will have to do some research.
These are very deep concepts.

Thread: std::find method seems to have the effect of altering the arguments!

Thread Tools

Search Thread

Display

std::find method seems to have the effect of altering the arguments!

Similar Threads

trouble passing arguments to method

No overload for method 'this' takes 2 arguments error

Arguments return method is not working for float

Find out which lib supports an method

trouble with arguments in main method

Tags for this Thread