C++ code,please C++ programer to give me some advise

naihe2010 · 10-14-2005, 08:44 PM

the class Date is a date struct ,it has a public method to make the date add 1.
but i think it's not good enough ,please give me some advise about the code.

Code:

#include <iostream>
using namespace std;

int am[]={0,31,28,31,30,31,30,31,31,30,31,30,31};

class Date{
private:
	int year,month,day;
public:
	Date(int,int,int);
	int increment(void);
	void disp(void);
};

Date::Date(int y,int m,int d){
	int t=0;
	if(!((y%4)||(!(y%4))&&(y%400))&&month==2){
		d--;
		t=1;
	}
	if(m<1||m>12||d<1||d>am[m]){
		cout<<"year,month or day data error!"<<endl;
		exit(1);
	}
	year=y;month=m;day=d+t;
}
int Date::increment(void){
	if(day<am[month])day++;
	else if(month==2){
                if(day==29||day==28&&((year%4)||!(year%100)&&(year%400))){
                        month++;
                        day=1;
                }
                else {
			day++;
		}
        }
	else if(month<12){
		month++;
		day=1;
	}
	else {
		year++;
		month=1;
		day=1;
	}
	return 1;
}
void Date::disp(void){
	cout<<"the date is:"<<year<<"/"<<month<<"/"<<day<<endl;
}

int main(void){
	int y,m,d;
	cout<<"Please input the date:"<<endl;
	cout<<"the year:"<<endl;
	cin>>y;
	cout<<"the month:"<<endl;
	cin>>m;
	cout<<"the day:"<<endl;
	cin>>d;
	Date a(y,m,d);
	a.increment();
	a.disp();
	return 0;
}

The problem is: I edit so many codes,and the programing style is not good ,please help me. I want to advanced my programing ability throngh this forum.

jonaskoelker · 10-14-2005, 10:18 PM

I'd change a few things:

(1) use `and', `or' and `not' instead of their (logical) equivalents (&&, ||, !)
(2) Use more spaces
(3) When you find an error, tell the caller (raise an exception or return an error value) instead of printing an error message (which btw. should go to cerr, not cout) and exiting.
(4) instead of dist, write an operator overloading for ostream <<;
(5) make am a static variable of the Date class.
(6) write a leap year predicate.

As for the actual increment algorithm, I'd say restructure it as follows:
(1) increment the day
(2) if the day is too big, set it to one and increment month
(3) similarly for month/year
It would at least make the code cleaner.

hth --Jonas

naihe2010 · 10-14-2005, 11:37 PM

Thank you for your advise.
I feel I known a lot of knowlage again.

But ,i have a little question:
that is,

how to write an operator overloading for ostream <<

I tried many times,but failed.

Could you give me some Prompts ?

jonaskoelker · 10-15-2005, 06:19 AM

Code:

ostream& operator <<(ostream& out, mytype myobj) {
    out << myobj.year << '/' << myobj.month << '/' << myobj.day;
}

naihe2010 · 10-15-2005, 11:26 AM

i edit it like this ,but return a lots of error.it can't be complied. the last error is :

ld returned 1 exit status

Where is the problem,the overloading predict,or not ?

Code:

#include <iostream>
using namespace std;

class Date{
private:
  static int am[13];
  int year,month,day;
public:
  Date(int,int,int);
  friend int leap(int);
  int increment(void);
  friend ostream& operator << (ostream&,Date);
};
	
ostream&  operator <<(ostream& out,Date date){
  out<<"The date is :"<<date.year<<"/"<<date.month<<"/"<<date.day<<endl;
  return out;
}
	
int leap(int y){
  if((y%100) and not(y%4) or not(y%400))return 1;
  return 0;
}
int Date::am[]={0,31,28,31,30,31,30,31,31,30,31,30,31};
Date::Date(int y,int m,int d){
  int t=0;
  if(m==2 and leap(y)){
    d--;
    t=1;
  }
  if(m<1 or m>12 or d<1 or d>am[m]){
    cout<<"year,month or day data error,can't save the date."<<endl;
    cout<<"Save a init date:2005/10/15"<<endl;
    y=2005;m=10;d=15;
  }
  year=y;month=m;day=d+t;
}
int Date::increment(void){
  if(month==2){
    if(day==29 or day==28 and not leap(year)){
      month++;
      day=1;
    }
    else {
      day++;
    }
  }
  else if(day<am[month])day++;
  else if(month<12){
    month++;
    day=1;
  }
  else {
    year++;
    month=1;
    day=1;
  }
  return 1;
}

int main(void){
  int y,m,d;
  cout<<"Please input the date:"<<endl;
  cout<<"the year:"<<endl;
  cin>>y;
  cout<<"the month:"<<endl;
  cin>>m;
  cout<<"the day:"<<endl;
  cin>>d;
  Date a(y,m,d);
  a.increment();
  cout<<a;
  return 0;
}

jonaskoelker · 10-15-2005, 01:38 PM

leap would be a lot prettier if it returned a bool instead of an int. `true' and `false' are boolean literals in C++.

Make leap static if it doesn't use instance variables.

Post the complete error message.

naihe2010 · 10-15-2005, 07:10 PM

Thank you very much,I rewrite it like this,and is it better now?

Code:

#include <iostream>
using namespace std;

class Date{
private:
  static int am[13];
  int year,month,day;
public:
  Date(int,int,int);
  friend bool leap(int);
  int increment(void);
  friend ostream& operator << (ostream&,Date);
};
	
ostream&  operator <<(ostream& out,Date date){
  out<<"The date is :"<<date.year<<"/"<<date.month<<"/"<<date.day<<endl;
  return out;
}

bool leap(int y){
  if((y%100) and not(y%4) or not(y%400))return true;
  return false;
}
int Date::am[]={0,31,28,31,30,31,30,31,31,30,31,30,31};
Date::Date(int y,int m,int d){
  int t=0;
  if(m==2 and leap(y)){
    d--;
    t=1;
  }
  if(m<1 or m>12 or d<1 or d>am[m]){
    cout<<"year,month or day data error,can't save the date."<<endl;
    cout<<"Save a init date:2005/10/15"<<endl;
    y=2005;m=10;d=15;
  }
  year=y;month=m;day=d+t;
}
int Date::increment(void){
  if(month==2){
    if(day==29 or day==28 and not leap(year)){
      month++;
      day=1;
    }
    else {
      day++;
    }
  }
  else if(day<am[month])day++;
  else if(month<12){
    month++;
    day=1;
  }
  else {
    year++;
    month=1;
    day=1;
  }
  return 1;
}

int main(void){
  int y,m,d;
  cout<<"Please input the date:"<<endl;
  cout<<"the year:"<<endl;
  cin>>y;
  cout<<"the month:"<<endl;
  cin>>m;
  cout<<"the day:"<<endl;
  cin>>d;
  Date a(y,m,d);
  a.increment();
  cout<<a;
  return 0;
}

i still have a problem with emacs. Every time I use emacs to edit something,what can I do to copy something to clipboard ?

jonaskoelker · 10-15-2005, 07:35 PM

Quote:

what can I do to copy something to clipboard ?

That depends on which `clipboard'. M-w puts the selection into the kill ring without removing it from the buffer (i.e `copy'), where C-w `cuts'.

If you want the X clipboard, you just select the text w. the mouse, and paste it w. mouse2 (middle btn.)

hth --Jonas

lowpro2k3 · 10-17-2005, 02:38 AM

I didnt analyze the whole code (its 2:30AM ;) ), but this jumped out at me:

Code:

ostream&  operator <<(ostream& out,Date date){
  out<<"The date is :"<<date.year<<"/"<<date.month<<"/"<<date.day<<endl;
  return out;
}

First - you're passing the Date object, 'date' by value. This is a small object (12 bytes most likely), but you should generally get in the habit of passing objects by reference. Apply the reference operator, so you're prototype looks like this:

Code:

ostream& operator <<(ostream& out, Date& date);

Now you're passing a 4 byte address instead of a 12 byte object. But now the function can make modifications and those changes will be reflected when the function exit. Since this is an example of a 'read-only' function, you should generally pass a constant reference to the object...

Code:

ostream& operator <<(ostream& out, const Date& date);

Now, with that out of the way... :)

This is a regular function, with no association to the Date class. I highly doubt it will compile, because you're requesting to access private Data class members (date.year, date.month, date.day). There are a few solutions, but this is best in my opinion, and I dont have time to explain all them ;)

A) Make it a class member function:

Code:

class Date
{
private:  // ...
public:  
  ostream& operator<<(ostream& out, const Date& date);
};

ostream& Date::operator<<(ostream& out, const Date& date)
{
  out<<"The date is :"<<date.year<<"/"<<date.month<<"/"<<date.day<<endl;
  return out;
}

Add the prototype to your class, and replace your operator<< function with the one I listed. Provide detailed error messages from the compiler if it doesn't work.

naihe2010 · 10-17-2005, 08:19 AM

I think the code should like this:

Code:

class Date
{
private:  // ...
public:  
    ostream& operator<<(ostream& out);
};

    ostream& Date::operator<<(ostream& out)
{
    out<<"The date is :"<<year<<"/"<<month<<"/"<<day<<endl;
    return out;
}

But it's still not right.Why ?

naihe2010 · 10-17-2005, 08:23 AM

OK,now.
The line72 should be:

Code:

a<<cout;

since it's right now,but why cout<<a it 's not right ?

Nylex · 10-17-2005, 08:37 AM

Quote:

Originally posted by naihe2010
I think the code should like this:

Code:

class Date { private: // ... public: ostream& operator<<(ostream& out); }; ostream& Date::operator<<(ostream& out) { out<<"The date is :"<<year<<"/"<<month<<"/"<<day<<endl; return out; }

But it's still not right.Why ?

Err, you need to supply another argument for operator<<, like in lowpro's example:

Code:

ostream& operator <<(ostream& out, Date& date);

Also, I'm not sure if you can declare that operator within the class itself (someone can correct me if I'm wrong here). If you do need to declare it outside of the class, then you will not be able to use the member variables (year, month, day) like that because they're private. You'll need to add methods that return them, eg.

Code:

int getMonth() { return month; }

jonaskoelker · 10-17-2005, 10:31 AM

IIRC, if you declare operators inside the class, the class instance will be the first (implicit) argument.

Example:

Code:

class foo {
    foo(int _x) {x = _x;}
    int x;
    foo operator + (foo other) {cout << x;}
}

a = foo(1);
b = foo(2);
a + b;

The output will be `1'.

However, you don't have to declare getters, you can instead declare the operator \keyword{friend}. When thing x is a friend of class y, x can access private fields of y instances.

hth --Jonas

lowpro2k3 · 10-17-2005, 11:30 AM

Quote:

Originally posted by jonaskoelker
However, you don't have to declare getters, you can instead declare the operator \keyword{friend}. When thing x is a friend of class y, x can access private fields of y instances.

hth --Jonas

Using getter's, and using friends were the other 2 methods I was going to explain, but didn't have the time to do

They would all work well. Generally I dont really have a preference, but I would probably use my method or getters. I usually tend to write all my operators within my class, just personal preference.