Is my code clean and concise?

[edwilke86]

I'm teaching myself, slowly, and came upon a problem in my book. It's "Jumping into C++" Chapter 16 (Recursions), problem one.


1. Write a recursive algorithm to compute the power function pow(x, y) = x^y


I have my code, and it works. So I'm just wondering if there is anyway to clean it up. I'm going for the barest minimum, no frill. Program takes 2 numbers, and raises the first number by the second.

I'm a high school math teacher and like to show my students my coding on mathematical problems to help them understand certain things. I need the code to be as clean as possible.

I know it's not messy, per-say, (thank you AStyle format) just wondering if there is any extraneous code in there I could remove while still keeping to the heart of the question, (using a recursive algorithm, instead of an iterative loop or something.)

/**       Dallas Wilke
 *        October 31, 2016
 *        Jumping in to C++
 *
 *        Chapter 16 - Problem One
 *   Write a recursive algorithm to compute the power function: pow( x,y ) = x^y
 *
 */


#include <iostream>

using namespace std;

int pow(int x, int y)
{
    if ( y == 1 )
    {
        return x;
    }
    return x * pow(x, y-1);
}
int main()
{
    int input1;
    int input2;
    cin >> input1 >> input2;
    cout << pow(input1, input2);
}

[AbstractionAnon]

That's about as concise as you can get.

If you want to write it a little more tersely, there is this using the ternary operator:

int pow (int x, int y)
{  return (y==1) ? x : x * pow(x,y-1);
}

[mbozzi]

Your code is not quite correct - your base-case isn't actually the base. (See below.)

Other then that -- assuming you make sure all the preconditions are enforced, it seems essentially fine.

Some notes on the definition of ::pow:
- since this is functional-style code it can be made constexpr with no pain (assuming C++14 or later).
- x and y can (should) be marked locally constant.
- Your name ::pow(int, int) can be conflated with ::std::pow(), especially given the using declaration using namespace std;.
- yours will not be called in favor of std::pow() if, for instance, one of the arguments is floating point. that's okay, except that the standard version has different semantics. You should change the name.
- Since you can't handle negative exponents, you better mark your exponent as unsigned and document why.
- For moderately large x and/or y the integer representation of the return value will overflow, resulting in undefined behavior (probably a nonsense answer).
- For sufficiently large y, the stack will overflow (unless your compiler optimizes accumulative-recursive functions) and crash your program.

The standard library doesn't provide an integer version of pow(). Even the version which accepts integers has their arguments promoted to double, because double supports floating point exceptions, infinities, and things. This helps the code return a defined result in more cases.

You can trade strict correctness for clarity - you have to choose where the border of "acceptably correct" is.
More advanced techniques can help introduce simplifying abstractions, but you stand the risk of getting the content lost in noise.

This fixes the the issue with the base-case:

int power(int const x, unsigned const y) { 
   if (y == 0) return 1; 
   else        return x * power(x, y - 1);
}