Jump to content

  • Log In with Google      Sign In   
  • Create Account

else if question


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
13 replies to this topic

#1 lightxbulb   Members   -  Reputation: 705

Like
0Likes
Like

Posted 22 May 2013 - 04:04 AM

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?



Sponsor:

#2 unbird   Crossbones+   -  Reputation: 4977

Like
2Likes
Like

Posted 22 May 2013 - 04:37 AM

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, 22 May 2013 - 04:44 AM.


#3 Felix Ungman   Members   -  Reputation: 1032

Like
3Likes
Like

Posted 22 May 2013 - 04:48 AM

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, 22 May 2013 - 04:54 AM.

openwar  - the real-time tactical war-game platform


#4 lightxbulb   Members   -  Reputation: 705

Like
0Likes
Like

Posted 22 May 2013 - 05:09 AM

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, 22 May 2013 - 05:10 AM.


#5 Felix Ungman   Members   -  Reputation: 1032

Like
2Likes
Like

Posted 22 May 2013 - 06:51 AM

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, 22 May 2013 - 06:52 AM.

openwar  - the real-time tactical war-game platform


#6 lightxbulb   Members   -  Reputation: 705

Like
0Likes
Like

Posted 22 May 2013 - 07:14 AM

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.



#7 Bacterius   Crossbones+   -  Reputation: 8870

Like
3Likes
Like

Posted 22 May 2013 - 08:00 AM

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)


The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#8 Felix Ungman   Members   -  Reputation: 1032

Like
2Likes
Like

Posted 22 May 2013 - 08:01 AM

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

openwar  - the real-time tactical war-game platform


#9 lightxbulb   Members   -  Reputation: 705

Like
2Likes
Like

Posted 22 May 2013 - 08:22 AM

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



#10 Álvaro   Crossbones+   -  Reputation: 13311

Like
2Likes
Like

Posted 22 May 2013 - 08:38 AM

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


#11 Álvaro   Crossbones+   -  Reputation: 13311

Like
1Likes
Like

Posted 22 May 2013 - 11:19 AM

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, 22 May 2013 - 11:20 AM.


#12 Felix Ungman   Members   -  Reputation: 1032

Like
0Likes
Like

Posted 22 May 2013 - 01:41 PM

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, 22 May 2013 - 01:41 PM.

openwar  - the real-time tactical war-game platform


#13 Álvaro   Crossbones+   -  Reputation: 13311

Like
1Likes
Like

Posted 22 May 2013 - 02:11 PM

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.



#14 Khatharr   Crossbones+   -  Reputation: 3001

Like
2Likes
Like

Posted 22 May 2013 - 04:33 PM

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.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS