446,156 Members | 1,026 Online
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 446,156 IT Pros & Developers. It's quick & easy.

# Help on OO leap year calculator

 P: n/a Hi!!! I need some help on a project I'm that calculates leap years; I'm getting errors that I have no idea what they mean; here's the code: #include #include using namespace std; class Year { public: Year(int y=0):year(new int) *year(y){} ~Year(){delete year;} Year(const Year& rhs)*year(rhs.*year){} Year operator=(const Year& rhs) { if(this!=&rhs) { *year=rhs->year; delete year; } else return this; } void SetYear(int y):year(new int),*year(y){} int GetYear()const{return *year;} bool IsLeapYear(const Year& y)const { if(y->year%400==0&&y->year%4==0) return true; else if(y->year%100) return false; else return false } friend ostream& operator<<(ostream& os,const Year& y); private: int* year; }; ostream& operator<<(ostream& os,const Year& y) { os << y.GetYear(); return os; } } int main() { int date; for(;;) { cout << "Enter a year: " << endl; cin >> date; Year year(date); if(year.IsLeapYear) cout << year << " is a leap year. " << endl; else cout << year << " is not a leap year. " << endl; bool cont; cout << "Continue?[1=yes|0=no]: " << endl; cin >> cont; if(cont==true)continue;else break; system("PAUSE"); return 0; } And as a side note, it's not letting me use constructor inizilization syntax. Can you help me (this isn't homework)? Thanks for your help!!! Sep 18 '05 #1
14 Replies

 P: n/a And what erros u get? Sep 18 '05 #2

 P: n/a And I'm sure this _is_ a school work ;> Sep 18 '05 #3

 P: n/a No it's not; my friend, Jonathon Mastron at BIOLA, asked me to it for him. My school work is trig stuff, not calendars and dates. Just help me please. Sep 18 '05 #4

 P: n/a OK, I fixed it; it compiles and runs now; here's the code: #include #include using namespace std; class Year { public: Year(int y=0): year(y){} ~Year(){} Year(const Year& rhs):year(rhs.year){} Year operator=(const Year& rhs) { if(this!=&rhs) year=rhs.year; return *this; } void SetYear(int y){year=y;} int GetYear(void)const{return year;} bool IsLeapYear(void)const { if(year%400==0&&year%4==0) return true; else if(year%100) return false; else return false; } friend ostream& operator<<(ostream& os,const Year& y); private: int year; }; ostream& operator<<(ostream& os,const Year& y) { os << y.GetYear(); return os; } int main() { int date; for(;;) { cout << "Enter a year: " << endl; cin >> date; Year year(date); if(year.IsLeapYear()) cout << year << " is a leap year. " << endl; else cout << year << " is not a leap year. " << endl; bool cont; cout << "Continue?[1=yes|0=no]: " << endl; cin >> cont; if(cont==true)continue;else break; } system("PAUSE"); return 0; } Anyway I can make it more efficient? And other than calculating leap years, what other uses can I have for this class? And what other ops should I overload? Thanks for the help!!! Sep 18 '05 #5

 P: n/a I'm trying to overload operator>>, but my program keeps aborting; here's the code for operator>>: istream& operator>>(istream& is,const Year& y) { is >> y.year; return is; } Why does it keep aborting? Sep 18 '05 #6

 P: n/a On 18 Sep 2005 15:35:16 -0700, "Protoman" wrote: I'm trying to overload operator>>, but my program keeps aborting;here's the code for operator>>:istream& operator>>(istream& is,const Year& y){is >> y.year; You are trying to modify a const value. In fact, the const qualifier is not appropriate for this kind of function. return is;}Why does it keep aborting? If it aborts at that region, then perhaps there might be a logic error in your code. Best to find some form of debugger. Sep 19 '05 #7

 P: n/a I fixed it and it works. Sep 19 '05 #8

 P: n/a to encapsulate, you should really make the leap year identification a non-friend, non-member, standalone function. like: bool is_leapyear(Year year){/*...*/} ben Sep 19 '05 #9

 P: n/a Protoman wrote: OK, I fixed it; it compiles and runs now; here's the code: .. bool IsLeapYear(void)const { if(year%400==0&&year%4==0) return true; else if(year%100) return false; else return false; } This function is clearly not correct. It will return false for nearly every year that is a leap year. The first clue that something is amiss, is the fact that it tests whether a year is divisible by 4 after it knows it is divisible by 400. Anyway, here is a corrected version: bool IsLeapYear() const { if ( year%4 ) // not divisible by 4 return false; if ( year%100 ) // not a century year return true; if ( year%400 ) // not a fourth century return false; return true; } Also, I would work on writing more readable code. The layout of the code in the original program is atrocious. Do spaces cost extra? Greg Sep 19 '05 #10

 P: n/a "benben" wrote in message news:43***********************@news.optusnet.com.a u... to encapsulate, you should really make the leap year identification a non-friend, non-member, standalone function. like: bool is_leapyear(Year year){/*...*/} ben How does that help anything? How does it provide encapsulation, when it's now an external function which has to know (and have access to) the internals of the Year class? And the year member of that object is a private variable, so if this is a non-friend, it can't possibly access the member. (Also, you're passing the Year object by value, which requires a copy for no apparent reason.) -Howard Sep 19 '05 #11

 P: n/a Your IsLeapYear method is wrong, e.g. 2004 will fail: 2004%400 == 4, 2004%4 == 0, so the first if will fail, 2004%100 == 4, so the else if will fail, so the method will return false. see http://www.faqs.org/faqs/calendars/faq/part1/ for the correct method. Sep 19 '05 #12

 P: n/a Howard wrote: "benben" wrote in message news:43***********************@news.optusnet.com.a u... to encapsulate, you should really make the leap year identification a non-friend, non-member, standalone function. like: bool is_leapyear(Year year){/*...*/} ben How does that help anything? Helps to keep the interface of Year small. That makes for fewer things to care about when you actually try to reap the advantages of encapsulation and set out to change the implementation. You will not need to even look at is_leapyear(). How does it provide encapsulation, when it's now an external function which has to know (and have access to) the internals of the Year class? Maybe I am imagination impaired, but why would is_leapyear() need to know "the internals" of Year. Don't you think that an ordinary call to a public member function like signed long GetYear() const; would suffice? And the year member of that object is a private variable, so if this is a non-friend, it can't possibly access the member. So you decided that a GetYear() function is inherently bad and that your Year class will never ever allow any client to actually figure out which year a given object of type Year represents. That, indeed, is very tight encapsulation. Admittedly, I have a hard time imagining a Year class to have interesting internals (the only invariant I can fathom is that the year cannot be 0). However, dreaming up an interface of Year that would not allow non-friend clients to tell whether the year is divisible by 4 and by 400 is beyond me. (Also, you're passing the Year object by value, which requires a copy for no apparent reason.) Good point. However, what would you think that sizeof(Year) should be? Best Kai-Uwe Bux Sep 19 '05 #13

 P: n/a "Kai-Uwe Bux" wrote in message news:dg**********@murdoch.acc.Virginia.EDU... Howard wrote: Admittedly, I have a hard time imagining a Year class to have interesting internals (the only invariant I can fathom is that the year cannot be 0). However, dreaming up an interface of Year that would not allow non-friend clients to tell whether the year is divisible by 4 and by 400 is beyond me. Looking more at his class, I'd have to agree. I see no reason for the class in the first place, really. A year is simply an integer. Why does it need functionality? Making the internal value a pointer to an integer is even more needless complexity. -Howard Sep 19 '05 #14

 P: n/a Protoman wrote: I fixed it and it works. C++ language issues aside, your leap year calculation is flawed. For example, according to your code, 1996 is not a leap year. Also, I'm not sure if a complete class isn't a little overkill, unless you do more with the year than figuring out if it's a leap year. This formula should work better: bool IsLeapYearFixed( const unsigned int year ) { if ( 0 != year % 4 ) return false; if ( 0 == year % 400 ) return true; if ( 0 == year % 100 ) return false; return true; } Cheers, Andre Sep 19 '05 #15

### This discussion thread is closed

Replies have been disabled for this discussion.