Sign in to follow this  
Followers 0
lightxbulb

else if question

13 posts in this topic

First of all, I am reading a book on C++ - there was this one example in it:

#include <iostream>
using namespace std;

class Date
{
private:
	int day, month, year;
public:
	Date(int inputDay, int inputMonth, int inputYear)
		: day(inputDay), month(inputMonth), year(inputYear){}


	bool operator== (const Date& input)
	{
		if( (day == input.day) && (month == input.month) && (year == input.year) )
			return true;
		else
			return false;
	}

	bool operator< (const Date& input)
	{
		if(year < input.year)
			return true;
		else if(month < input.month)
			return true;
		else if(day < input.day)
			return true;
		else 
			return false;
	}

	bool operator<= (const Date& input)
	{
		if(this->operator== (input))
			return true;
		else 
			return this->operator< (input);
	}

};

int main()
{
	Date d1(19 , 4, 2013);
	Date d2(19, 5, 2012);
	cout << (d1<d2) << endl;

	cin.ignore();
	return 0;
}

 

 

Btw it's me that changed the main a little - just to show what a ridiculous output I get.

Is it me or is the inequality operator coded wrong?

	bool operator< (const Date& input)
	{
		if(year < input.year)
			return true;
		else if(month < input.month)
			return true;
		else if(day < input.day)
			return true;
		else 
			return false;
	}

 

If it were me I'd code it like this:

	bool operator< (const Date& input)
	{
		if(year < input.year)
			return true;
		else if(year > input.year)
			return false;
		else //when year == input.year
		{
			if(month < input.month)
				return true;
			else if(month > input.month)
				return false;
			else //when month == input.month
			{
				if(day < input.day)
					return true;
				else 
					return false;
			}
		}
	}

 

Is it the book's example that is wrong or is it me missing something crucial?

0

Share this post


Link to post
Share on other sites
Nope, you're perfectly right. Whenever you compare sequences you only go deeper if the current elements are equal.

PS: Check if the book has an (online) errata and/or if you're using the latest edition. Edited by unbird
2

Share this post


Link to post
Share on other sites

What book is that? The "if (…) return true; else return false;" looks a bit, ehm, naive too...

 

I usually use something like the following pattern for less-than:

 

bool operator< (const Date& input) const
{
    int diff = year - input.year;
    if (diff == 0) diff = month - input.month;
    if (diff == 0) diff = day - input.day;
    return diff < 0;
}
Edited by Felix Ungman
3

Share this post


Link to post
Share on other sites

This book:

http://www.amazon.com/Sams-Teach-Yourself-One-Hour/dp/0672335670

I don't think it's the best one out there but I like it.

 

The "if (…) return true; else return false;" looks a bit, ehm, naive too...

Idk about that, you allocate an additional variable of type int on the stack, perform 3 subtractions and assignments and all in all the code doesn't look straight-forward(to me at least - especially if you consider what's the goal of the operator) - probably just a matter of taste.

Edited by lightxbulb
0

Share this post


Link to post
Share on other sites

Another option to improve readability is to skip the elses and curlies.

bool operator<(const Date& input) const
{
    if (year < input.year) return true;
    if (year > input.year) return false;
    if (month < input.month) return true;
    if (month > input.month) return false;
    if (day < input.day) return true;
    return false;
}

 

An advantage of the diff approach is that it easily can be used with strings and other data structures.

if (diff == 0) diff = strcmp(title, input.title); 

 

You can also write a strcmp-like function to use as a primitive for the operators.

int date_cmp(const Date& a, const Date& b) { … }

bool operator<(const Date& input) const { return date_cmp(*this, input) < 0; }
bool operator>(const Date& input) const { return date_cmp(*this, input) > 0; }
bool operator<=(const Date& input) const { return date_cmp(*this, input) <= 0; }
bool operator>=(const Date& input) const { return date_cmp(*this, input) >= 0; }
bool operator==(const Date& input) const { return date_cmp(*this, input) == 0; }
bool operator!=(const Date& input) const { return date_cmp(*this, input) != 0; }
Edited by Felix Ungman
2

Share this post


Link to post
Share on other sites

I get what you mean - that's why I said it's probably a matter of taste.

Concerning the elses and curlies - I'm not really sure which's better - using a return as a break or having a few additional curlies and elses.

I consider cutting off a function with just a return (or doing the same in a loop with break) "bad", I think that a few elses and braces promote readability over a return or a break.

This example here is trivial:

bool operator<(const Date& input) const
{
    if (year < input.year) return true;
    if (year > input.year) return false;
    if (month < input.month) return true; // the programmer has to take into account that year and input.year were evaluated
    // if you move month before year you'd get a problem
    if (month > input.month) return false;
    if (day < input.day) return true;
    return false;
}

 

Still I can't perceive this as good practice - of course it works - however (for me at least) this is sure to be more confusing than a code with a few additional elses and braces.

As I said it's a matter of taste - building the logical flow in a function around returns may cause me trouble in a bigger and more complex project(I'd have a big issue if I have to find a bug and I've based the logical flow inside my functions around return type breaks). 

 

About your 2nd point - I guess it's good, in the end it all depends on what goals you have for your function/class though.

0

Share this post


Link to post
Share on other sites

I get what you mean - that's why I said it's probably a matter of taste.

Concerning the elses and curlies - I'm not really sure which's better - using a return as a break or having a few additional curlies and elses.

I consider cutting off a function with just a return (or doing the same in a loop with break) "bad", I think that a few elses and braces promote readability over a return or a break.

This example here is trivial:

bool operator<(const Date& input) const
{
    if (year < input.year) return true;
    if (year > input.year) return false;
    if (month < input.month) return true; // the programmer has to take into account that year and input.year were evaluated
    // if you move month before year you'd get a problem
    if (month > input.month) return false;
    if (day < input.day) return true;
    return false;
}

 

Still I can't perceive this as good practice - of course it works - however (for me at least) this is sure to be more confusing than a code with a few additional elses and braces.

As I said it's a matter of taste - building the logical flow in a function around returns may cause me trouble in a bigger and more complex project(I'd have a big issue if I have to find a bug and I've based the logical flow inside my functions around return type breaks). 

 

About your 2nd point - I guess it's good, in the end it all depends on what goals you have for your function/class though.

 

If it were up to me I would've written it like this:

 

bool operator<(const Date& input) const
{
  if (year != input.year) return (year < input.year);
  if (month != input.month) return (month < input.month);
  if (day != input.day) return (day < input.day);
  return false; // same date
 }

The control flow is clear and concise - check each date field in order of significance, return with the correct comparison if they differ. If the code makes it to the last line, the two dates are equal and hence we return false (as we're overloading the strictly less operator). This might look better in a general purpose date comparison function, though.

 

Early return is a pattern I enjoy very much as it simplifies assertions I need to keep track of throughout my code. Trivial edge cases can be dispatched quickly instead of endlessly dragging them along the entire function block and writing redundant conditions everywhere. Of course arcane code is bad, but when it's easy to read, is a well-understood idiom, or is in general intuitive, there is no problem.

 

The main issue I have with your "return else" concept is that while it is syntactically consistent, it doesn't add any value to the code except an extra keyword, scope and indentation level. Unless the code inside that "if" condition is freakishly complicated (which it shouldn't and isn't here) it doesn't take a genius to see that anything entering that block will return, hence the "else" is redundant and is just there to eat up horizontal space. And as a real life analogy, would you tell someone this?


Please go buy food, then, when you have bought the food, make dinner. Or, if there was no food at the store, don't make dinner.

 

 

No, that is a horribly redundant sentence. What you would say is:


Please go buy food and then make dinner.

 

 

There's no need to specify that you need to actually buy the food first before making dinner. Nor is there a need to explicitly tell whoever you're talking to that he shouldn't make dinner if he couldn't buy any food, since he can't. It's self-evident. Just as it is in the comparison code. There is no need to keep on comparing the date if the years differ. You're done. There isn't even a need to check what the rest of the function is like. Remember an important programming adage: DRY. Don't Repeat Yourself. That applies to code as well as documentation.

 

That said this stuff comes with experience in writing code and you will eventually develop your own style that you find elegant, effective and productive. But just because it doesn't look intuitive to you now doesn't mean it's bad. However, it's good that you're asking questions and discussing why things are the way they are, it shows that you reflect on the code you write. I think we were all puzzled by some code at some point in time (and everyone is always in a perpetual state of learning when it comes to programming anyway, even if they won't admit it)

3

Share this post


Link to post
Share on other sites

Good questions.

 

The problem with multiple returns and breaks is usually not that there are multiple exit points, but that the code is so complex that those exit points are hard to see. If the code is simple enough you'll easily see all the dependencies in it. Also, evaluation order usually matters (more often than the other way around).

 

But you're right that much of readability is a matter of taste. It's easier to read what you are used to read.

 

Btw, wouldn't the single-return version be something like this: (Not sure I prefer this one though.)

bool operator<(const Date& input) const
{
     return year < input.year || (year == input.year && (month < input.month || (month == input.month && day < input.day)));
}
2

Share this post


Link to post
Share on other sites

Thanks guys!

I won't avoid so much organizing my program flow around return/breaks now.

I still think that this is valid:

building the logical flow in a function around returns may cause me trouble in a bigger and more complex project(I'd have a big issue if I have to find a bug and I've based the logical flow inside my functions around return type breaks). 

But I guess it really depends on what is "complex code" - this of course was a trivial example so maybe it really wouldn't hurt organizing things with return.

As you said:

this stuff comes with experience in writing code

I just don't want to end up with some bad habits/style when programming - because as I've come to see - different books/tutorials promote different style of coding and I guess it's all about getting the best out of each book/tutorial, while ignoring the "bad practices". That's why a forum as this one is especially helpful in such situations.

So thanks once again guys - I really appreciate it! smile.png

2

Share this post


Link to post
Share on other sites

My own style is very similar to Bacterius's:

bool operator<(const Date& input) const {
  if (year != input.year)
    return year < input.year;

  if (month != input.month)
    return month < input.month;

  return day < input.day;
}
2

Share this post


Link to post
Share on other sites

Oh, here's an alternative that a lot of people won't like, but I am starting to like more and more:

 

bool operator<(const Date &input) const {
  return year  != input.year  ? year  < input.year
       : month != input.month ? month < input.month
       :                        day   < input.day;
}

 

What I like about it is that it's as easy to read as a if-elseif-else construction, but it is clear from the beginning that the only thing we are doing is returning a value.

Edited by Álvaro
1

Share this post


Link to post
Share on other sites

If we're listing alternatives, there's yet another one, inspired by C#'s nullables and ?? operator.

    bool* less_than_order(int x, int y)
    {
        static bool _true = true;
        static bool _false = false;
        if (x == y) return nullptr;
        return x < y ? &_true : &_false;
    }

    bool operator<(const Date& input) const
    {
        static bool _false = false;
        return *(less_than_order(year, input.year)
              ?: less_than_order(month, input.month)
              ?: less_than_order(day, input.day)
              ?: &_false);
    }

 

Not that I would use it in code, but it's an interesting variation on this theme.

Edited by Felix Ungman
0

Share this post


Link to post
Share on other sites

Yet another solution (for this very specific case): Use an int to represent a date in yyyymmdd format. This works great to sort and it's trivial to see the value of a date in the debugger:

(gdb) bt
#0  day_of_the_week (yyyymmdd=20130522) at kk.cpp:4
#1  0x00000000004008df in main () at kk.cpp:11

 

This is a feature that pretty much no other representation of dates has.

1

Share this post


Link to post
Share on other sites

The "single exit point" restriction always bothered me. I understand that it can be confusing in a large or complex function and result in later changes being incorrect, but I think the proper solution to that is not writing overly large or complex functions. Because of this I write things like AABB checks thuswise:

 

bool checkCollide(RECT &thisRect, RECT &otherRect) {
  if(thisRect.left   > otherRect.right)  {return false;}
  if(thisRect.right  < otherRect.left)   {return false;}
  if(thisRect.top    > otherRect.bottom) {return false;}
  if(thisRect.bottom < otherRect.top)    {return false;}
  return true;
}

It's true that you have to engage your brain for a moment to see that it works, but that's true for other solutions as well. This way is compact and straightforward. Once you understand the method, any potential errors are immediately visible.

2

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0