# OCaml rev_int function

With my original code I kept getting Error: Parse error: [expr level ;] expected after "in" (in [expr]) on the line let numDigits = numDigits - 1 in

Original:

let rec rev_int num =
if num / 10 == 0 then
num
else
let temp = num mod 10 in

let numDigits = String.length(string_of_int num) - 1 in

if num < 0 then
let numDigits = numDigits - 1 in
else
let numDigits = numDigits + 0 in

let num = (num - temp) / 10 in
temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num


With variations of:

if num < 0 then
let numDigits = numDigits - 1 in;
else
let numDigits = numDigits + 0 in;


if num < 0 then
let numDigits = numDigits - 1 in
else begin
let numDigits = numDigits + 0 in end


I revised the code and now it works, but I was wondering if there was a way to do it with nested if and less redundancy.

Revised:

let rec rev_int num =
if num / 10 == 0 then
num
else
let temp = num mod 10 in

let numDigits = String.length(string_of_int num) - 1 in

if num < 0 then
let numDigits = numDigits - 1 in
let num = (num - temp) / 10 in
temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num
else
let numDigits = numDigits + 0 in
let num = (num - temp) / 10 in
temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num


It seems that in both branches you're doing the same calculation, so might want to begin by pushing the conditional in, obtaining:

let rec rev_int num =
if num / 10 == 0 then
num
else
let temp = num mod 10 in

let numDigits = String.length(string_of_int num) - 1 in

let numDigits = numDigits - (if num < 0 then 1 else 0) in
let num = (num - temp) / 10 in
temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num


Just an initial thought to get you started on improving the code.

Nested ifs are just a minor problem with this function. A much bigger problem is that the algorithm is poor.

It appears that you want to reverse the digits of an integer. To do that, you are doing all kinds of costly conversions, like string_of_int, float_of_int, and int_of_float. In particular, converting an int to a string actually involves many operations behind the scenes — just so that you can figure out how many places to left-shift your number!

What you want is a recursive algorithm that only manipulates the least-significant digits of numbers.

let rev_int num =
let rec rev_int' n m = match n with
| 0 -> m
| n -> rev_int' (n / 10) (10 * m + (n mod 10))
in rev_int' num 0


From the revised version:

Factor the common code from both branches:

let rec rev_int num =
if num / 10 == 0 then
num
else
let temp = num mod 10 in

let numDigits = String.length(string_of_int num) - 1 in

let numDigits = numDigits - (if num < 0 then 1 else 0) in
let num = (num - temp) / 10 in
temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num


(num - temp) / 10 = num / 10 when temp = num mod 10:

let rec rev_int num =
if num / 10 == 0 then
num
else
let temp = num mod 10 in

let numDigits = String.length(string_of_int num) - 1 in

let numDigits = numDigits - (if num < 0 then 1 else 0) in
let num = num / 10 in
temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num


Inline temp definition, as it is a terrible name anyway, and avoid rebinding the same name multiple times:

let rec rev_int num =
if num / 10 == 0 then
num
else
let numDigits = String.length(string_of_int num) - 1
- (if num < 0 then 1 else 0) in
num mod 10 * int_of_float(10.0 ** float_of_int numDigits)
+ rev_int (num / 10)


Maybe more idiomatic ocaml: use snake_case and pattern matching (with is also more efficient: you don't recalculate n / 10 twice) :

let rec rev_int num =
match num / 10 with
| 0 -> num
| q ->
let num_digits = String.length (string_of_int num) - 1
- (if num < 0 then 1 else 0) in
num mod 10 * int_of_float (10.0 ** float_of_int num_digits)
+ rev_int q