Checking if two numbers have the same sign

Question

Is there a more elegant way than the following to check if two numbers have the same sign?

 bool sameSign(int num1, int num2)
 {
 if (num1 > 0 && num2 < 0)
 return false;
 if (num1 < 0 && num2 > 0)
 return false;
 return true;
 }

It could also be written as the not case of all of this (e.g. check the true cases and return false at the end).

By design, the two integers will never contain a zero value.

• JD.B: If by design the two integers will never contain a zero value, then should there be a check/exception for that (to make sure that that case never hits)?
• John Demetriou: this is checked before the function
• Eric Lippert: Then it should be asserted in the function.
• Eric Lippert: The lesson to learn from this question and the (some deleted) answers is that trying to make code "more elegant" by taking clear, debugged, working code and applying some weird trick often introduces a bug. Though there are small ways your code should be improved, it works and it is very clear to the reader. Many of the attempts below that try to use algebraic or representational properties are either arcane, wrong, or at least hard to see that they are right. I would stick with the code you have.
• bdsl: I think its potentially dangerous to use a line break without a curly bracket after an if statement, since someone can come along later and introduce a second line of code which will always be executed but may look like its conditional. That's part of how apple's famous 'goto fail' bug happened.

Answer 1

First up, your code has bugs. What if one of the numbers is 0? 0 should be positive, but your code treats it as negative in some tests, and positive in others. When comparing against 0 you should use >=, not just >.

The actual code is quite readable, though, and the performance is probably not horrible. I would recommend a single return statement though. A single return statement is easier if you check for "the same sign", and not "opposite signs".

Take your code:

bool sameSign(int num1, int num2)
{
 if (num1 > 0 && num2 < 0)
 return false;
 if (num1 < 0 && num2 > 0)
 return false;
 return true;
}

Fix the 0-handling, and you have:

bool sameSign(int num1, int num2)
{
 return num1 >= 0 && num2 >= 0 || num1 < 0 && num2 < 0
}

Now, that's pretty good, and I would happily "pass" that in a code review, but, can you do some tricks?

The simplest (code wise) is to use XOR:

return (num1 ^ num2) >= 0

That compares the bits, and if they are the same, it sets the resulting bit to 0. If the sign bits are the same, the resulting sign-bit is 0, and thus a positive (or 0) value.

• Gentian Kasa: That's a usage of XOR I'd never have thought of.
• Darrel Hoffman: Note that the XOR trick will only work for integer type variables. If you wanted a version that works on floats or doubles, you'd use one of the other versions.
• Justin: Why should 0 be positive? There is no reason for 0 to be considered positive. Mathematically, 0 is neither positive nor negative.
• rolfl: @Justin - note that the requirement is to check for the "same sign", not "positive" or "negative". As it happens, binary 0 has the same sign as positive integers, despite any mathematical applications/theory
• Justin: I didn't think that the intent was to think of binary integers, but rather integer. I would treat it as num1 == 0 && num2 == 0 || num1 < 0 && num2 < 0 || num1 > 0 && num2 > 0. I guess it depends on the interpretation.

Answer 2

Why not compare the Booleans themselves?

return ((num1 < 0) == (num2 < 0));

This treats zero as "positive". For a stricter interpretation that considers zero to be neither positive nor negative, consider:

return (((num1 == 0) == (num2 == 0)) &&
 ((num1 < 0) == (num2 < 0)));

(Edit hat tips @MichaelS, @holroy)

• John Demetriou: That's also a good approach yes
• mc0e: This gets my vote. I expect it's marginally less efficient than the XOR trick, but it says exactly what it means, and therefore is much more readable.
• some1 here: @mc0e, also XOR does not work for floats. So, that is the most efficient option in the float case.

Answer 3

Yes, there is a more elegant way to do this by

• adding accessibility modifier to the method
• use PascalCase casing for naming the method
• naming the method HasSameSign
• using the Math.Sign() method

private static bool HasSameSign(int num1, int num2)
{
 return Math.Sign(num1) == Math.Sign(num2);
}

• 200_success: The Math.Sign() method produces a different result if either num1 or num2 is 0. Your interpretation of the meaning of "same sign" is more conventional, so arguably the original code is buggy, or at least the method is poorly named.
• kojiro: I have the impression that your and my answers offer a textbook differentiation between OOP and FP.
• Matthew Steeples: @200_success if you want 0 to be either positive or negative than you can tweak the code to be return Math.Abs(Math.Sign(num1) - Math.Sign(num2)) > 1.

Answer 4

You can shift right, you will end up with -1 or zero, for negative and positive respectively. (Arithmetic shift) This will propagate the most significant bit (sign bit) yielding 0xFFFFFFFF (-1) or 0x00000000 (0).

return num1 >> 31 == num2 >> 31;

• John Demetriou: more explanation please. If they are both positive what will I get? if they are both negative, what will I get? Also explain why the shifting works
• John Demetriou: Thanks for the edit. Upvoted your answer. But the other one seems a lot more simpler and readable by others so I accepted that one. Although yours is language agnostic
• 200_success: This technique treats 0 just like positive numbers, which is not what I consider expected behaviour, nor does it reproduce the (arguably dubious) behaviour of the original code.
• fdomn-m: I like this approach, but it does rely on the underlying implementation of int, which should generally not be relied upon. This could not be directly used for eg long. Now it's pretty clear that c# will (most) likely always be a System.Int32 but some may recall there was plenty of wailing and gnashing of teeth when vb7(vb.net) was introduced and 2 byte ints were suddenly 4 bytes!! omg So in x years when C#128 is introduced and your successor's graddaughter ports your mission critical code without understanding bitshift, don't come running to us when the world economy collapses...
• sara: @freedomn-m could be guarded against by using sizeof (int) or by explicitly using System.Int32 in the method signature (although since the size of int is part of the C# specification it won't be changed so it's a non-issue. I'm more concerned of how voodoo this solution is and would prefer more readability).

Answer 5

The thing that is missing is a comment above the function that specifies exactly what the function does for inputs that are zero: Zero values could be (a) interpreted as being positive, they could be (b) interpreted as signless (having the same sign as any number), or (c) the behaviour of the function could be undefined for zero inputs.

That needs to be written down, as a comment that is part of the function. If you had done that, then in case (b) or (c) your code would have been a fine implementation, and in case (a) it would have a severe bug.

Now anyone who blindly multiplies the numbers (severe risk of overflow, and the same problem handling zeroes), or shifting tricks (and adding assumptions about what right shift of negative numbers does, and assumptions that int = 32 bits): Don't try that at work! Do it as home as much as you like, but not where readability and code quality is asked for.

Answer 6

The code can written in one line:

return (num1 <= 0 && num2 <= 0) || (num1 >= 0 && num2 >= 0);

This preserves the original behavior of the code, which, as @200_success commented, is different than the conventional meaning of same sign.

• John Demetriou: The shift method treats zero as positive. So does math.sign?
• Gentian Kasa: I think there's a mistake there. In order to preserve the original code one could also use De Morgan's laws, and the end result should become return (num1 <= 0 || num2 >= 0) && (num1 >= 0 || num2 <= 0);
• Spike: @GentianKasa It's equivalent. Both check whether the numbers or both non-positive or both non-negative. Mine is "sum of products", i.e. it tests for the two cases where the sign is equal. The original code (and your one line) is "product of sums", i.e. it works by eliminating the two cases where the sign is not equal (one positive and one negative).
• Gentian Kasa: Yeah, you're right. Taking a better look at it they're equivalent. My bad

Answer 7

A simple way using simple math. The resulting sign of a division will be positive if the two numbers have the same sign, and negative if they are distinct:

 bool sameSign(int num1, int num2)
 {
 num2 = num2==0 ? 1 : num2;
 return ((float)num1 / num2) > 0;
 }

The first line num2 = num2==0 ? 1 : num2; depends on what zero means for you:

1. zero is neither positve nor negative
2. or zero is both
3. or you think it can be eiter +0 and -0.

In this case, num2 = num2==0 ? 1 : num2; zero is always positive (case 3, +0). You can change it to -0 by switching 1 to -1.

If you think case 2 is the real deal, the first line would be num2 = num2==0 ? num1 : num2;, this way the result of the division will be always 1.

Finally, for case 1, num2 = num2 == 0 ? (num1 == Int32.MinValue ? 1 : num1 * -1) : num2;, you might say it must always return false, since "something (some sign) equals nothing (no sign)" is always false. You can achieve that by switching the num1 sign having the division be the original num1 / num1 with switched sign, which is always < 0.

Note in C# the division by two integers return an integer (rounded). By casting to float forces the result to be float. Without it, the result would be an int, and if the num2 is larger enough it would round to 0, both if it was approaching from the negative or positive side.

• John Demetriou: Again. No zero value will exist. For the rest, I think this is similar to the multiplication answer just different perspective. Right?
• Eric Lippert: @JohnDemetriou: No. The way division preserves sign and the way multiplication preserves sign are different because multiplication can overflow.
• Eric Lippert: @Zukki: Is your answer involving multiplication by -1 correct for the case where multiplying by -1 does not change the sign?
• Zukki: @EricLippert yes, in that case the value of num2 is set to the value of num1 with its opposite sign. Then, the division is num1/ (num1 with oposite sign), and that is always < 0
• Eric Lippert: I think you are not following me. MinValue is a negative number which, when multiplied by -1, is still a negative number. Does your proposed algorithm which multiplies by -1 still work for this value?

Answer 8

 bool sameSign(int num1, int num2)
 {
 if (num1 > 0 && num2 < 0)
 return false;
 if (num1 < 0 && num2 > 0)
 return false;
 return true;
 }

First line (prototype) should be (if you want to check doubles):

bool sameSign(long double num1, long double num2)

(note that both must be floating-point numbers)

3rd and 5th lines are completely wrong (3rd line takes 0 as num1 as negative, 5th takes 0 as num2 as negative).
So they should respectively be:

if (num1 >= 0 && num2 < 0)

and

if (num1 < 0 && num2 >= 0)

to have right results. But I recommend 1 return only:

return num1>=0&&num2>=0||num1<0&&num2<0

so that the function would just be:

bool sameSign(long double num1, long double num2)
{
 return num1>=0&&num2>=0||num1<0&&num2<0;
}

or a one-liner:

bool sameSign(long double num1,long double num2){return num1>=0&&num2>=0||num1<0&&num2<0;}

or a two-liner:

bool sameSign(long double num1,long double num2){return num1>=0&&num2>=0||num1<0
&&num2<0;}

Answer 9

I want to introduce one more alternative way for checking that two numbers has same sign.

The method of choice of course is to The method code is here, and a fat explanation follows.

(Only for Int32; other Int types, you should change the number 31 to the bit size of the integer type minus 1 , added on warning of Dmitry Rubanovich)

return System.Convert.ToBoolean(~(num0>>31 ^ num1>>31) & 1);

Using the following two facts, we can build a complete bitwise check (without any comparison) for the signs to be equal.

computers use a method called the two's complement for storing negative numbers.in that form the most significant bit in binary form is 1 if a number is negative and it is 0 if the number is positive.
we can access an 32bit integer's most significant bit with following snippet:

num0>>31 

that shifts it 31 bits to right, ignoring every bit except the leftmost one(sign indicator bit :-) )

the XOR( ^ ) operator return 1 if two bits are not equal and returns 0 if both bits are equal. if we use the "~" ( "binary one's complement operator" that basically flips bits) on the XOR results (called XNOR) and extract the least significant bit of it( using a logical "and" with integer 1 will do the trick) we get 1 if the bits are equal and 0 if the bits are not equal in C# we have to explicitly convert int to boolean so, combining two facts above we get:

return System.Convert.ToBoolean(~(num0>>31 ^ num1>>31) & 1);

The bitwise code returns true if at least one of the numbers is zero.

• John Demetriou: This question is the classic example of: "we have 100 developers, so we have at least 200 opinions" :-P, good answer though
• Dmitry Rubanovich: Your answer is just short of perfect. If you want something which works regardless of how large your numbers, but does rely on the fact that 2 ints are the same size, it's return (num0 ^ num1) >=0
• FazeL: @DmitryRubanovich I'm not an native speaker, what do you mean with "is just short of perfect", I, personally advise(but doesn't act :-D ) to do it with math.sign or a library whenever it is possible, using if for verbosity and do not relay on hacks until it is necessary. the answer as I said is just a show off of bitwise tricks for peoples interested. and I believe the way you put is generally more safe thus more useful. my method would only be useful when datatypes are 32 bits strictly and the function calls are enormous.
• Dmitry Rubanovich: @FazelL, using a shift by a fixed number of bits forces to only consider integers which 32 bits wide. By using XOR instead of multiply, you eliminate the possibility of overflow. Which makes your answer an improvement on a few previous answers. But then your mind naturally reaches for another bit-wise operation to finish. If you use a regular comparison instead, you get an answer which works for any two integers of the same bit-width.
• MAG: This answer doesn't even compile. In C# you can't implicitly convert int to bool.

Answer 10

Multiply the two number together. If the sign of the result is positive, then the sign was the same. If the sign of the result is negative, then the signs were different.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
namespace SignsSameProject
{
 public partial class Form1 : Form
 {
 public Form1()
 {
 InitializeComponent();
 }
 private void button1_Click(object sender, EventArgs e)
 {
 bool x;
 x = SignsAreTheSame(5, 2);
 if (x == true)
 MessageBox.Show("True");
 else
 MessageBox.Show("False");
 }
 private bool SignsAreTheSame(int x, int y)
 {
 long z;
 z = x * y;
 if (z < 0)
 return false;
 else
 return true;
 }
 }
}

• Rick Davin: Test your answer against these 2 positive numbers (2, int.MaxValue) and again with a positive and negative (2, int.MinValue).

Answer 11

How about:

return (num1 < 0) == (num2 < 0);

This checks if 2 numbers have the same sign, in one line

The logic in in this one liner is clearer and is probably faster than if statements

• pacmaninbw: Welcome to the Code Review Community. The point of the code review site is to help programmers improve their coding skills by making insightful observations about the original code. We do not provide alternate solutions.
• Peilonrayz: Please explain how and why your code is better than the OP's. Thanks.
• user244080: @pacmaninbw he is looking for a 'more elegant way'
• Toby Speight: Welcome to Code Review! We prefer answers that make new observations; your answer merely duplicates kojiro's existing answer, and doesn't add any value here. Please find something else within the code that can be improved.
• user244080: @TobySpeight I apologize, I did not see that answer