Thread: std container issue (copy semantics?)

  1. #1
    Kiss the monkey. CodeMonkey's Avatar
    Join Date
    Sep 2001
    Posts
    937

    std container issue (copy semantics?)

    Hello,
    I'm creating a money-management program, and have run into a few related problems regarding std::vector s of my objects. Let's just dump it out:
    Windows Vista (on my new laptop ), Dev-C++
    error-spawning lines are in red.
    Code:
    C:/Dev-Cpp/include/c++/3.4.2/bits/vector.tcc: In member function `transaction& transaction::operator=(const transaction&)':
    C:/Dev-Cpp/include/c++/3.4.2/bits/vector.tcc:238:   instantiated from `void std::vector<_Tp, _Alloc>::_M_insert_aux(__gnu_cxx::__normal_iterator<typename _Alloc::pointer, std::vector<_Tp, _Alloc> >, const _Tp&) [with _Tp = transaction, _Alloc = std::allocator<transaction>]'
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_vector.h:564:   instantiated from `void std::vector<_Tp, _Alloc>::push_back(const _Tp&) [with _Tp = transaction, _Alloc = std::allocator<transaction>]'
    
    account.cpp:17:   instantiated from here
    C:/Dev-Cpp/include/c++/3.4.2/bits/vector.tcc:238: error: non-static const member `const Date transaction::date_created', can't use default assignment operator
    
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h: In function `const _Tp& std::__median(const _Tp&, const _Tp&, const _Tp&) [with _Tp = transaction]':
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:2484:   instantiated from `void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<transaction*, std::vector<transaction, std::allocator<transaction> > >, _Size = int]'
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:2555:   instantiated from `void std::sort(_RandomAccessIterator, _RandomAccessIterator) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<transaction*, std::vector<transaction, std::allocator<transaction> > >]'
    account.cpp:18:   instantiated from here
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:90: error: passing `const transaction' as `this' argument of `bool transaction::operator<(const transaction&)' discards qualifiers
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:91: error: passing `const transaction' as `this' argument of `bool transaction::operator<(const transaction&)' discards qualifiers
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:93: error: passing `const transaction' as `this' argument of `bool transaction::operator<(const transaction&)' discards qualifiers
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:97: error: passing `const transaction' as `this' argument of `bool transaction::operator<(const transaction&)' discards qualifiers
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:99: error: passing `const transaction' as `this' argument of `bool transaction::operator<(const transaction&)' discards qualifiers
    Code:
    double account::transact(Date date_in, const std::string & name_in, double amount_in, bool nosort)
    {
           bal += amount_in;
           registry.push_back(transaction(date_in,name_in,amount_in,bal));       
           if(!nosort) std::sort(registry.begin(),registry.end());       
           return bal;
    }
    I think that is has to do with my operator <
    Code:
    bool transaction::operator< (const transaction & other) { return date < other.date; }
    
    //..... elsewhere......
    
    inline bool Date::operator< (const Date & x)
    {
           return y == x.year() ? ( m == x.month() ? d < x.day() : m < x.month() ) : y < x.year();
           //years are not same -> compare years
           //years are the same -> (months are not same -> compare months)
           //                      (months are the same -> compare days)
    }
    And, finally... the involed objects.....
    Code:
    class Date
    {
     public:
            enum Spec { today, yesterday, tomorrow };
            enum Month { jan = 1, feb, mar, apr, may, jun, jul, aug, sep, oct, nov, dec };
            
            Date(Month month_in = Month(0), int day_in = 0, int year_in = 0);
            Date(Spec s);
            std::string str();
            int day() const { return d; }
            int month() const { return m; }
            int year() const { return y; }
            bool operator < (const Date & x);
     private:
             int d;
             int y;
             Month m;
    };
    
    extern Date::Month & operator--(Date::Month &m);
    extern Date::Month & operator++(Date::Month &m);
    Code:
    class transaction
    {
    	public:
    		transaction( const Date& date_in = Date(), const std::string& name_in = std::string(), 
                         double amount_in = 0.0,       double new_bal_in = 0.0, 
                         const Date& date_created_in = Date(Date::today) );
    		~transaction() {}
    		
    		std::string file_rep();
    		std::string display_rep();
    		bool operator< (const transaction & other) { return date < other.date; }
    		
            Date date;
            const Date date_created;
            std::string name;
            double amount;
            double new_bal;
    };
    
    extern std::ostream & operator << (std::ofstream & s, transaction & tran);
    extern std::ostream & operator << (std::ostream & s, transaction & tran);
    Code:
    class account
    {
    	public:
    		account(const std::string & name_in);
    		~account();
    		
    		double transact(Date date_in, const std::string & name_in, double amount_in, bool nosort = false);
    		std::string display(unsigned int num = 0); //shows the last num transactions.
    		                                   //0 means show all.
            const std::string & name() const { return n; }
            double balance() const { return bal; }
             
    	private:
            std::vector<transaction> registry;
            double bal;
            std::string n;
    };
    Any help would be greatly appreciated.
    Last edited by CodeMonkey; 06-27-2007 at 05:13 PM. Reason: extra tag
    "If you tell the truth, you don't have to remember anything"
    -Mark Twain

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    The problem does not have to do with operator<. You have a non-static const member in your transaction class. This means that the default assignment operator won't work, because it does memberwise copy assignment and you cannot assign to a const variable. You need a valid copy assignment operator to use vector.

    This can be annoying. I don't know of a good solution. You could make date_created not const anymore. You might also consider not holding transaction objects in your vector.

  3. #3
    Kiss the monkey. CodeMonkey's Avatar
    Join Date
    Sep 2001
    Posts
    937
    Thank you. Discarding the const qualifiers fixed many of the issues. However, sort() is still complaining about opertor<, saying that there is no match. I changed the function's argument form a const reference to a plain reference, but that was a shot in the dark. What might create this:
    Code:
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:90: error: no match for 'operator<' in '__a < __b'
    transaction.h:22: note: candidates are: bool transaction::operator<(transaction&)
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:91: error: no match for 'operator<' in '__b < __c'
    transaction.h:22: note: candidates are: bool transaction::operator<(transaction&)
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:93: error: no match for 'operator<' in '__a < __c'
    transaction.h:22: note: candidates are: bool transaction::operator<(transaction&)
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:97: error: no match for 'operator<' in '__a < __c'
    transaction.h:22: note: candidates are: bool transaction::operator<(transaction&)
    C:/Dev-Cpp/include/c++/3.4.2/bits/stl_algo.h:99: error: no match for 'operator<' in '__b < __c'
    transaction.h:22: note: candidates are: bool transaction::operator<(transaction&)
    "If you tell the truth, you don't have to remember anything"
    -Mark Twain

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    Didn't see that. You are right, it is a small problem with the operator<. If you make it a member function it should be const (add const to the end of the declaration and definition).

    You might also consider making it a non-member with two reference to const parameters.

  5. #5
    Kiss the monkey. CodeMonkey's Avatar
    Join Date
    Sep 2001
    Posts
    937
    Excellent. Now the only issue is that the sort() causes a crash if called a second time. I don't see how that can happen (it's not like I'm managing my own memory)... am I using it incorrectly?
    Code:
    double account::transact(Date date_in, const std::string & name_in, double amount_in, bool nosort)
    {
           bal += amount_in;
           registry.push_back(transaction(date_in,name_in,amount_in,bal)); //this works
           if(!nosort) std::sort(registry.begin(),registry.end()); //this works ONCE ONLY
           return bal;
    }
    Code:
    bool operator < (const transaction & a, const transaction & b) {  return a.date < b.date;  }
    Code:
    bool operator < (const Date & a, const Date & b)
    {
         return a.year() == b.year() ? ( a.month() == b.month() ? a.day() < b.day() : a.month() < b.month() ) : a.year() < b.year();
    }
    "If you tell the truth, you don't have to remember anything"
    -Mark Twain

  6. #6
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    I don't see where it would crash, either. Your classes all seem copyable. Your operator< functions all look like they return false on equal (which they should). I don't see any use of pointers or something else that can get easily corrupted. You aren't even storing references to objects anywhere that I can see.

    Is registry corrupted somehow? Can you tell where in the sort code the crash occurs? Can you output the contents of registry just before the crash? Are you sure that the account object that is being called when transact is called is valid?

    Do you use operator[] with registry anywhere? If so, try changing it to at() at least during development.

    Your Month enum has no 0 value, but you default to Month(0) in your Data constructor. Shouldn't you be defaulting to a valid Month? It's possible that sort is creating a default Data object for some reason.

  7. #7
    Kiss the monkey. CodeMonkey's Avatar
    Join Date
    Sep 2001
    Posts
    937
    It crashes right at the sort, though I can't tell wherein because I can't get the debugger working. The only place std::vector<transaction> registry is even referenced to is in account::transact(), and account::display(), and I assure you display() is not the problem. So at least I'm not crazy.
    What, exactly, do most implementations of std::sort(std::vector<T>::iterator,std::vector<T>: :iterator) do?

    *edit* Date::m is initialized to Month(0) by default, denoting a null state. What is a date if the creator does not specify? Not a logical date, I hope: just a logical data type.
    *edit* The vector's state before a second sort is ship-shape. How irritating...
    Last edited by CodeMonkey; 06-27-2007 at 10:15 PM.
    "If you tell the truth, you don't have to remember anything"
    -Mark Twain

  8. #8
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    But I think Month(0) might be undefined behavior. Either add a null_month to the enum or initialize to january or something. Does it crash if you use Month(1) instead?

    Honestly I can't think of anything else based on the code given. If nobody else sees anything then beyond getting the debugger working your next step is to start paring down the code until you get a small example that still reproduces the crash.

  9. #9
    Kiss the monkey. CodeMonkey's Avatar
    Join Date
    Sep 2001
    Posts
    937
    AHAH! Bad object code. A project-wide rebuild fixed the problem. Thanks again for your help, Daved.

    *edit* Stroustrup used a Month(0) in his class Date example, without having enumerated the value.
    "If you tell the truth, you don't have to remember anything"
    -Mark Twain

  10. #10
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> *edit* Stroustrup used a Month(0) in his class Date example, without having enumerated the value.

    You are right. In section 4.8 he explains that the range of the enumeration starts at 0 (assuming all non-negative enumerators) and that you can convert any integer in the range of the enumeration to the enumeration type even if no enumerator actually equals that value. So Month(0) is perfectly acceptable.

    Glad your code is working now.

  11. #11
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    On an unrelated design note, I don't think operator < makes sense for transactions. You should do this:
    Code:
    struct before
    {
      bool operator ()(const transaction &lhs, const transaction &rhs)
      {
        return lhs.date < rhs.date;
      }
    };
    
    // ...
    std::sort(registry.begin(), registry.end(), before());
    The question with < is, what does it compare? The dates the transactions took place? The dates the transactions where created? Or the amount of money in the transactions? The last one is, from a linguistic point of view, the most logical: a big transaction is one where a lot of money is transferred, thus a smaller ("lesser") transaction is one where less money is transferred.
    But the point is, it's not clear what < means. You should use an explicit comparator instead.

    Oh, and I think transaction should use getters and setters.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  12. #12
    Kiss the monkey. CodeMonkey's Avatar
    Join Date
    Sep 2001
    Posts
    937
    Good point. Thank you.
    About the getters and setters, I figured since transactions are used only by class Account, that it didn't matter, assumung that transactions are insulated from the user (which they are). Also, considering the microscopic scale of this project, I found plain old open data easier. Why would you use setters and getters?
    "If you tell the truth, you don't have to remember anything"
    -Mark Twain

  13. #13
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    For the same reasons as always. More flexibility and better control.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. List copy semantics efficiency
    By dudeomanodude in forum C++ Programming
    Replies: 31
    Last Post: 05-24-2008, 05:05 PM
  2. calling copy constructor from template
    By Ancient Dragon in forum C++ Programming
    Replies: 3
    Last Post: 09-28-2005, 01:54 PM
  3. dynamic memory alloccation & returning objects
    By haditya in forum C++ Programming
    Replies: 8
    Last Post: 04-21-2005, 11:55 PM
  4. Linked list copy constructor issue
    By Craptastic! in forum C++ Programming
    Replies: 1
    Last Post: 08-03-2003, 08:30 PM
  5. Copy Constructor Help
    By Jubba in forum C++ Programming
    Replies: 2
    Last Post: 11-07-2001, 11:15 AM