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.

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.

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&)

---

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.

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();
}

---

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.

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...

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.

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.

>> *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.

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.

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?

For the same reasons as always. More flexibility and better control.