What is wrong with this code?(returning an item in the vector)

Discussion in 'C++' started by Sims, Jan 20, 2004.

  1. Sims

    Sims Guest

    Hi,

    Here is the code that .NET does not seem to like, but as far as i can see it
    is valid C++ code.
    Am i wrong?

    ....
    // vector headers
    ....

    struct MYSTRUCT
    {
    int m_iSomething;
    };

    typedef std::vector< MYSTRUCT, std::allocator<MYSTRUCT > > MYSTRUCT_VECTOR;

    ....
    // Within class
    ....

    MYSTRUCT_VECTOR g_MyVector;

    int CMyClass::FindSomeThing( int pos )
    {
    // return item
    }

    MYSTRUCT * CMyClass::GetStructure( int iSomething)
    {
    // Find the item in the vector
    int pos = FindSomeThing( iSomething );
    // Anything found
    if( pos <0 ) return NULL;
    // return what we have
    return (g_MyVector.begin()+pos);
    }

    The code above should work i think but .NET 2002 gives me an error? What am
    i missing?

    Many thanks

    Sims
     
    Sims, Jan 20, 2004
    #1
    1. Advertisements

  2. Sims

    Sims Guest

    Sorry, the error .NET gives me is that there is a conversion error in my
    return value.

    Sims
     
    Sims, Jan 20, 2004
    #2
    1. Advertisements

  3. Your return value is of type vector<...>::iterator not of MYSTRUCT*. Just
    because vector iterators are most often implemented as ordinary pointers
    doesn't mean iterators and pointers are interchangeable.
     
    Hendrik Belitz, Jan 20, 2004
    #3
  4. Sims

    Howard Guest

    To follow up on that...

    if you want to return pointers to MYSTRUCT, then you can always actually
    STORE pointers to MYSTRUCT, instead of the objects themselves. In that
    case, you dereference the iterator to get your pointer. (And remember that
    you'll need to delete the objects pointed to when you're done with the
    vector.)

    I'm not sure if you can return a pointer to an object in a vector when
    you're storing the objects themselves, however. Someone else would have to
    answer that. (I've only used vectors where I store the pointers,
    personally.)

    -Howard
     
    Howard, Jan 20, 2004
    #4
  5. Using operator+() on random access iterators is defined in the C++ Standard,
    so I don't think you should use std::advance() here.
     
    Hendrik Belitz, Jan 20, 2004
    #5
  6. AFAIK you can do the following:

    vector<object> v;
    ....
    vector<object>::iterator it = v.begin();
    ....
    object* ptr = &(*it);

    So this works, altough I think working with references or even copies
    (Insteas of returning a pointer) may be a better way.
     
    Hendrik Belitz, Jan 20, 2004
    #6
  7. Sims

    Chris Theis Guest

    The reason for the error is that what you actually return is a
    vector::iterator and not (necessarily) a pointer to MYSTRUCT. Although
    vector iterators are very often implemented in terms of ordinary pointers
    they still have an implementation specific type, which can be totally
    different from an ordinary pointer. The portable way to solve this problem
    is the following for example:

    return &(*g_MyVector.begin());

    However, I'm amazed that .NET 2002 complains as VC++ 6.0 has no problem with
    your approach (although I wouldn't recommend it!).

    Cheers
    Chris
     
    Chris Theis, Jan 20, 2004
    #7
  8. Sims

    Sharad Kala Guest

    This is probably because Iterators for vector class on your implementation are
    not implemented as pointers.
    The same code (after some changes) however compiles fine on Comeau online.

    So, to be safe use MYSTRUCT_VECTOR::iterator as the return type instead of
    MYSTRUCT *.
    Also use std::advance instead of '+' on iterators.

    Best wishes,
    Sharad
     
    Sharad Kala, Jan 20, 2004
    #8
  9. Sims

    Sharad Kala Guest

    True :)
    But to make the code generic for other containers one should be using advance.
    Since here it's a vector using '+' is justified but otherwise if the container
    itself is generic then it's preferable to use advance.

    Best wishes,
    Sharad
     
    Sharad Kala, Jan 20, 2004
    #9
  10. Sims

    Sims Guest

    Hi,
    Sorry, what do you not recommend? VC++6 or My approach?, if it is my
    approach what would you do instead?

    Sims
     
    Sims, Jan 20, 2004
    #10
  11. Generally your approach is not portable and is also no good programming
    style. I would recommend to strictly distinct between iterators and
    pointers. Or see pointers as iterators, but not the other way round.
     
    Hendrik Belitz, Jan 20, 2004
    #11
  12. Sims

    Chris Theis Guest

    My posting might be a little misleading regarding what I do not recommend.
    If you prefer to leave the code as it is returning a pointer to MYSTRUCT
    then you should do it as I showed above:

    return &(*g_MyVector.begin() );

    On the other hand you can also change the return type to
    MYSTRUCT_VECTOR::iterator and leave the return statement as it is. The way
    you implemented it works under VC++ 6.0 and Comeau although it is not
    required to do so and it is not portable. Hence, I'd recommend to change
    either the return type or the return statement.

    Regards
    Chris
     
    Chris Theis, Jan 20, 2004
    #12
  13. Sims

    News 2 Guest

    You must be very careful if you return pointers to objects stored in a
    vector. The vector can and likely
    will reassign the objects to other locations when you add data to the
    vector. Storing pointers rather than
    object is a better solution but doesn't utilize the capabilities of vectors
    and STL in general. Obviously you must look at how you are using the array
    and decide what is appropriate.

    James
     
    News 2, Jan 20, 2004
    #13
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.