7

Simple question - In c++, what's the neatest way of getting which of two numbers (u0 and u1) is the smallest positive number? (that's still efficient)

Every way I try it involves big if statements or complicated conditional statements.

Thanks, Dan

Here's a simple example:

bool lowestPositive(int a, int b, int& result)
{
    //checking code
    result = b;
    return true;
}


lowestPositive(5, 6, result);
Tall Jeff
  • 9,834
  • 7
  • 44
  • 61
Dan
  • 33,953
  • 24
  • 61
  • 87

16 Answers16

16

If the values are represented in twos complement, then

result = ((unsigned )a < (unsigned )b) ? a : b;

will work since negative values in twos complement are larger, when treated as unsigned, than positive values. As with Jeff's answer, this assumes at least one of the values is positive.

return result >= 0;
Doug Currie
  • 40,708
  • 1
  • 95
  • 119
  • Clever. A bit of a kludge, but still clever. – Tall Jeff Dec 26 '08 at 15:56
  • @tvanfosson: if both arguments are negative then result is negative and the function returns false therefore the concrete value of the `result` doesn't matter in this case. – jfs Dec 26 '08 at 17:03
  • @Shmoopty: Could you provide an example when the function gives an incorrect answer? – jfs Dec 26 '08 at 17:09
  • @JF -- I see now. Still the fact that I was able to miss that makes it a "magic" solution to me. I tend to avoid cleverness and go with the easier to read solution. In this case, the actual text of the answer threw me off. Another lesson: read the code, not the documentation! :-) – tvanfosson Dec 26 '08 at 17:37
  • 1
    It is unclear whether OP considers positive or non-negative numbers therefore I've posted an answer for positive numbers i.e., excluding zero. http://stackoverflow.com/questions/393885/neatest-fastest-algorithm-for-smallest-positive-number#394101 – jfs Dec 26 '08 at 19:22
  • @tvanfosson: I agree. Readability counts. – jfs Dec 26 '08 at 19:25
  • It's better to encapsulate the method inside a function, this makes it more useful to reuse it. (Even better, you could generalize it with templates) Besides that you are losing efficiency by type casting, the author should make sure a and b are initially unsigned. – Tamara Wijsman Dec 27 '08 at 01:26
  • that shouldn't lose any efficiency. it would just use an unsigned compare operation at the assembler level probably. in twos' complement, the bit patterns won't change when casting from int to unsigned int. – Johannes Schaub - litb Dec 27 '08 at 02:36
12

I prefer clarity over compactness:

bool lowestPositive( int a, int b, int& result )
{
   if (a > 0 && a <= b) // a is positive and smaller than or equal to b
      result = a;
   else if (b > 0) // b is positive and either smaller than a or a is negative
      result = b;
   else
      result = a; // at least b is negative, we might not have an answer

   return result > 0;  // zero is not positive
}
tvanfosson
  • 524,688
  • 99
  • 697
  • 795
  • Cool. I prefer this clarity too :) – Gant Dec 26 '08 at 16:37
  • And no constraints on the sign of the inputs. :) – tvanfosson Dec 26 '08 at 16:41
  • almost perfect, it's probably always faster to return true or false right after the assignments (gcc actually tests the result again as it is now, even though it's already known whether the result is positive when the return statement is reached) – mjy Dec 27 '08 at 01:19
  • As @HugoHeden's answer points out, this fails if `a > 0` but `b < 0`. His answer is basically this with templates, and an extra condition in the first `if()`. – Peter Cordes Apr 12 '16 at 00:42
  • @HugoHeden Nice, catch - fixed. – tvanfosson Apr 12 '16 at 06:14
3
unsigned int mask = 1 << 31;
unsigned int m = mask;
while ((a & m) == (b & m)) {
  m >>= 1;
}
result = (a & m) ? b : a;
return ! ((a & mask) && (b & mask));

EDIT: Thought this is not so interesting so I deleted it. But on the second thought, just leave it here for fun :) This can be considered as a dump version of Doug's answer :)

PolyThinker
  • 5,152
  • 21
  • 22
  • This code being modded down is ridiculous. It does what it's meant to do, and rather cleverly I might add. So I vote +1. – mstrobl Dec 26 '08 at 16:40
  • The topic is inviting to play around a little. Why must we always be so earnest, Princess? :) Hey, it's clever code. Why waste it into the -1 nirvana? – mstrobl Dec 26 '08 at 16:47
  • To cope with integers not 32 bits wide, what about a mask like (1 << std::numeric_limits::digits) or without templates ((1 << (CHAR_BIT * (sizeof (int)-1))) << (CHAR_BIT-1)) – Johannes Schaub - litb Dec 27 '08 at 00:44
  • Right. It's not meant to cope with different sizes. Another way to do it is (unsigned int) -1 ^ ( (unsigned int) -1 >> 1 ), which should work on almost any C compiler. – PolyThinker Dec 27 '08 at 02:10
3

Might get me modded down, but just for kicks, here is the result without any comparisons, because comparisons are for whimps. :-)

bool lowestPositive(int u, int v, int& result)
{
  result = (u + v - abs(u - v))/2;
  return (bool) result - (u + v + abs(u - v)) / 2;
}

Note: Fails if (u + v) > max_int. At least one number must be positive for the return code to be correct. Also kudos to polythinker's solution :)

mstrobl
  • 2,381
  • 14
  • 16
  • 1
    Yes, it's called maths. And it's intended to be read tongue-in-cheek. :) – mstrobl Dec 26 '08 at 17:28
  • @dmckee: Maybe this reads better as: "midpoint=(u+v)/2;dist=abs(u-v)/2; result = midpoint-dist;" --which shows that result does always get the smaller of the two values (irrespective of what the signs are) but that the "return (bool)result-(midpoint+dist);" returns true unless u=v: so it's wrong :P – ShreevatsaR Dec 26 '08 at 18:16
  • @ShreevatsaR: I must admit, I did not test the code. Great analysis! ;) The incorrect return value boils down to an error in my thinking. return result > 0 is correct, but unfortunately has one of those whimpy comparisons in it. – mstrobl Dec 26 '08 at 18:32
  • 1
    Don't mistake me. I'm suitably impressed. Deliberately using the wrong tool is cool, but only if it is really awkward. This one qualifies. – dmckee --- ex-moderator kitten Dec 26 '08 at 23:57
  • Aside from the accepted answer I find this one the easiest to understand :) – Lamar Latrell Apr 12 '16 at 06:20
3

Here's a fast solution in C using bit twiddling to find min(x, y). It is a modified version of @Doug Currie's answer and inspired by the answer to the Find the Minimum Positive Value question:

bool lowestPositive(int a, int b, int* pout)
{
  /* exclude zero, make a negative number to be larger any positive number */
  unsigned x = (a - 1), y = (b - 1);    
  /* min(x, y) + 1 */
  *pout = y + ((x - y) & -(x < y)) + 1; 
  return *pout > 0;
}

Example:

/** gcc -std=c99 *.c && a */
#include <assert.h>
#include <limits.h>
#include <stdio.h>
#include <stdbool.h>

void T(int a, int b) 
{           
  int result = 0;   
  printf("%d %d ", a, b);       
  if (lowestPositive(a, b, &result))    
    printf(": %d\n", result);       
  else              
    printf(" are not positive\n");  
}

int main(int argc, char *argv[])
{
  T(5, 6);
  T(6, 5);
  T(6, -1);
  T(-1, -2);

  T(INT_MIN, INT_MAX);
  T(INT_MIN, INT_MIN);
  T(INT_MAX, INT_MIN);
  T(0, -1);
  T(0, INT_MIN);
  T(-1, 0);
  T(INT_MIN, 0);
  T(INT_MAX, 0);
  T(0, INT_MAX);
  T(0, 0);

  return 0;
}

Output:

5 6 : 5
6 5 : 5
6 -1 : 6
-1 -2  are not positive
-2147483648 2147483647 : 2147483647
-2147483648 -2147483648  are not positive
2147483647 -2147483648 : 2147483647
0 -1  are not positive
0 -2147483648  are not positive
-1 0  are not positive
-2147483648 0  are not positive
2147483647 0 : 2147483647
0 2147483647 : 2147483647
0 0  are not positive
Community
  • 1
  • 1
jfs
  • 399,953
  • 195
  • 994
  • 1,670
2

I suggest you refactor the function into simpler functions. Furthermore, this allows your compiler to better enforce expected input data.

unsigned int minUnsigned( unsigned int a, unsigned int b )
{
   return ( a < b ) ? a : b;
}

bool lowestPositive( int a, int b, int& result )
{
   if ( a < 0 && b < 0 )  // SO comments refer to the previous version that had || here
   {
       return false;
   }

   result = minUnsigned( (unsigned)a, (unsigned)b );  // negative signed integers become large unsigned values
   return true;
}

This works on all three signed-integer representations allowed by ISO C: two's complement, one's complement, and even sign/magnitude. All we care about is that any positive signed integer (MSB cleared) compares below anything with the MSB set.

This actually compiles to really nice code with clang for x86, as you can see on the Godbolt Compiler Explorer. gcc 5.3 unfortunately does a much worse job.

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
antik
  • 5,282
  • 1
  • 34
  • 47
  • That returns the highest positive. – Doug Currie Dec 26 '08 at 15:47
  • a = -1 b = 1 should return 1 but doesn't – pro Dec 26 '08 at 16:10
  • According the problem set, I'm working on the set of integers > 0 both because my if statement enforces that and because that's what the OP asked for. Furthermore, what I'm trying to demonstrate is that by refactoring the function into smaller pieces, the logic becomes far simpler. – antik Dec 26 '08 at 16:22
  • 1
    The sense of your first if statement is incorrect. It will return false if both a and b are positive, not false if a and b are negative. – tvanfosson Dec 26 '08 at 16:35
  • Interesting use of unsigned to make negative numbers into numbers higher than the largest signed int. This requires a comment, though! – Peter Cordes Apr 12 '16 at 00:46
2

This will handle all possible inputs as you request.

bool lowestPositive(int a, int b, int& result)
{
    if ( a < 0 and b < 0 )
        return false

    result = std::min<unsigned int>( a, b );
    return true;
}

That being said, the signature you supply allows sneaky bugs to appear, as it is easy to ignore the return value of this function or not even remember that there is a return value that has to be checked to know if the result is correct.

You may prefer one of these alternatives that makes it harder to overlook that a success result has to be checked:

boost::optional<int> lowestPositive(int a, int b)
{
    boost::optional<int> result;
    if ( a >= 0 or b >= 0 )
        result = std::min<unsigned int>( a, b );
    return result;
}

or

void lowestPositive(int a, int b, int& result, bool &success)
{
    success = ( a >= 0 or b >= 0 )

    if ( success )
        result = std::min<unsigned int>( a, b );
}
Drew Dormann
  • 59,987
  • 13
  • 123
  • 180
2

tons of the answers here are ignoring the fact that zero isn't positive :)

with tricky casting and tern:

bool leastPositive(int a, int b, int& result) {
    result = ((unsigned) a < (unsigned) b) ? a : b;
    return result > 0;
}

less cute:

bool leastPositive(int a, int b, int& result) {
    if(a > 0 && b > 0)
        result = a < b ? a : b;
    else
        result = a > b ? a : b:
    return result > 0;
}
wowest
  • 1,974
  • 14
  • 21
1

No cleverness, reasonable clarity, works for ints and floats:

template<class T>
  inline
  bool LowestPositive( const T a, const T b, T* result ) {
  const bool b_is_pos = b > 0;
  if( a > 0 && ( !b_is_pos || a < b ) ) { 
    *result = a;
    return true;
  }
  if( b_is_pos ) {
    *result = b; 
    return true;
  }
  return false;
}
  • Note that 0 (zero) is not a positive number.
  • OP asks for dealing with numbers (I interpret this as ints and floats).
  • Only dereference result pointer if there is a positive result (performance)
  • Only test a and b for positiveness once (performance -- not sure if such a test is expensive?)

Note also that the accepted answer (by tvanfosson) is wrong. It fails if a is positive and b is negative (saying that "neither is positive"). (This is the only reason I add a separate answer -- I don't have reputation enough to add comments.)

Community
  • 1
  • 1
Hugo Heden
  • 183
  • 1
  • 6
1

Three lines with the use (abuse?) of the ternary operator

int *smallest_positive(int *u1, int *u2) {
    if (*u1 < 0) return *u2 >= 0 ? u2 : NULL;
    if (*u2 < 0) return u1;
    return *u1 < *u2 ? u1 : u2;
}

Don't know about efficiency or what to do if both u1 and u2 are negative. I opted to return NULL (which has to be checked in the caller); a return of a pointer to a static -1 might be more useful.

Edited to reflect the changes in the original question :)

bool smallest_positive(int u1, int u2, int& result) {
    if (u1 < 0) {
        if (u2 < 0) return false; /* result unchanged */
        result = u2;
    } else {
        if (u2 < 0) result = u1;
        else result = u1 < u2 ? u1 : u2;
    }
    return true;
}
pmg
  • 106,608
  • 13
  • 126
  • 198
1

Hack using "magic constant" -1:

enum
{
    INVALID_POSITIVE = -1
};

int lowestPositive(int a, int b)
{
    return (a>=0 ? ( b>=0 ? (b > a ? a : b ) : INVALID_POSITIVE ) : INVALID_POSITIVE );
}

This makes no assumptions about the numbers being positive.

Marcin
  • 12,245
  • 9
  • 42
  • 49
1

Pseudocode because I have no compiler on hand:

////0 if both negative, 1 if u0 positive, 2 if u1 positive, 3 if both positive
switch((u0 > 0 ? 1 : 0) + (u1 > 0 ? 2 : 0)) {
  case 0:
    return false; //Note that this leaves the result value undef.
  case 1:
    result = u0;
    return true;
  case 2:
    result = u1;
    return true;
  case 3:
    result = (u0 < u1 ? u0 : u1);
    return true;
  default: //undefined and probably impossible condition
    return false;
}

This is compact without a lot of if statements, but relies on the ternary " ? : " operator, which is just a compact if, then, else statement. "(true ? "yes" : "no")" returns "yes", "(false ? "yes" : "no") returns "no".

In a normal switch statement after every case you should have a break;, to exit the switch. In this case we have a return statement, so we're exiting the entire function.

Aaron Friel
  • 1,065
  • 5
  • 11
  • 1
    -1: Sorry, but this just isn't the type of code I'd want to see in production. There are more concise ways to get the results the OP wants, and the comment above your switch statement indicate that you're aware the code is so "clever" that its no longer readable. – Juliet Dec 26 '08 at 16:37
1

With all due respect, your problem may be that the English phrase used to describe the problem really does hide some complexity (or at least some unresolved questions). In my experience, this is a common source of bugs and/or unfulfilled expectations in the "real world" as well. Here are some of the issues I observed:

  • Some programmers use a naming convention in which a leading u implies unsigned, but you didn't state explicitly whether your "numbers" are unsigned or signed (or, for that matter, whether they are even supposed to be integral!)

  • I suspect that all of us who read it assumed that if one argument is positive and the other is not, then the (only) positive argument value is the correct response, but that is not explicitly stated.

  • The description also doesn't define the required behavior if both values are non-positive.

  • Finally, some of the responses offered prior to this post seem to imply that the responder thought (mistakenly) that 0 is positive! A more specific requirements statement might help prevent any misunderstanding (or make it clear that the issue of zero hadn't been thought out completely when the requirement was written).

I'm not trying to be overly critical; I'm just suggesting that a more precisely-written requirement will probably help, and will probably also make it clear whether some of the complexity you're concerned about in the implementation is really implicit in the nature of the problem.

joel.neely
  • 30,725
  • 9
  • 56
  • 64
1
uint lowestPos(uint a, uint b) { return (a < b ? a : b); }

You are looking for the smallest positive, it is be wise to accept positive values only in that case. You don't have to catch the negative values problem in your function, you should solve it at an earlier point in the caller function. For the same reason I left the boolean oit.

A precondition is that they are not equal, you would use it like this in that way:

if (a == b)
  cout << "equal";
else
{
  uint lowest = lowestPos(a, b);
  cout << (lowest == a ? "a is lowest" : "b is lowest");
}

You can introduce const when you want to prevent changes or references if you want to change the result. Under normal conditions the computer will optimize and even inline the function.

Tamara Wijsman
  • 12,198
  • 8
  • 53
  • 82
0

My idea is based on using min and max. And categorized the result into three cases, where

  • min <= 0 and max <= 0
  • min <= 0 and max > 0
  • min > 0 and max > 0

The best thing is that it's not look too complicated. Code:

bool lowestPositive(int a, int b, int& result)
{
    int min = (a < b) ? a : b;
    int max = (a > b) ? a : b;

    bool smin = min > 0;
    bool smax = max > 0;

    if(!smax) return false;

    if(smin) result = min;
    else result = max;

    return true;
}
Gant
  • 29,661
  • 6
  • 46
  • 65
0

After my first post was rejected, allow me to suggest that you are prematurely optimizing the problem and you shouldn't worry about having lots of if statements. The code you're writing naturally requires multiple 'if' statements, and whether they are expressed with the ternary if operator (A ? B : C) or classic if blocks, the execution time is the same, the compiler is going to optimize almost all of the code posted into very nearly the same logic.

Concern yourself with the readability and reliability of your code rather than trying to outwit your future self or anyone else who reads the code. Every solution posted is O(1) from what I can tell, that is, every single solution will contribute insignificantly to the performance of your code.

I would like to suggest that this post be tagged "premature optimization," the poster is not looking for elegant code.

Aaron Friel
  • 1,065
  • 5
  • 11