The unchecked IllegalArgumentException is lovely, thank you.
Very nice diagnostic error.
Referring to the function's name as log_10 seems like a typo.
You might consider including the value of x in the message,
for the benefit of some poor maintenance engineer
chasing down a difficult-to-reproduce bug.
obscure comparison
while (x / 10 > 0) {
That seems a slightly cryptic way to communicate Author's Intent.
Consider rephrasing as (x >= 10), which I think is what you meant.
Alternatively, mention a loop variant in a comment,
to emphasize we're predicting the effect of the x /= 10 assignment.
meaningful identifiers
double r = a;
Neither one of these is a great name.
And it's unclear why we abandoned a in favor of r.
For a local variable to be single letter is not necessarily a problem.
But the meaning, and usually the "mental pronunciation", should be clear,
e.g. a comment hinting that "the Radix starts out as the Approximation".
(No, I don't believe Author intended either of those nouns.
IDK, perhaps the Root of some unspecified equation we're solving?)
If you had cited a reference, perhaps a wikipedia URL,
then the Gentle Reader could come up to speed by tracing
that reference's variable names, which you preserved,
down into your source code.
Absent comments or a cited reference, we likely want more
than single-character names for these.
OTOH names like x or i are perfect as-is.
The frac identifier is well-chosen.
extract helper
The point of that while loop is to compute
very special values of x and of a.
Tell us the meaning of those values.
You could use a comment, but it would
be much better to break out a tiny helper function.
It will have an informative name.
We can /** document */ its return value(s).
We can separately unit test it.
magic number
...; i < 25; ...
I imagine there's some special relationship between
a power of two and a power of ten.
But I'm not seeing it.
You have to spell it out for us,
minimally with a comment,
preferably with a helpfully named MANIFEST_CONSTANT.
Here's what I see, but it appears to be off
by a factor of about a million:
$ python -c 'print(f"{2 ** 64}\n{int(1e19)}")'
18446744073709551616
10000000000000000000
width of x
frac /= 2;
Oh, wait!
We're cycling through 25 bits?!?
How is that relevant?
Don't we want at least 53 bits of
significand?
Tell us the meaning of what you're computing.
Minimally you have to document that we don't
correctly process all input integers, as we only
offer limited precision.
Or put another way,
we don't compute what the Math library computes,
so we can't recycle its javadoc
documentation
for this function.
(Well, plus a signature difference. And we substitute an exception for a NaN return,
a change which I feel is perfectly reasonable.)
Wow, that's a surprise I didn't see coming!
Note that Math.log10 computes a different function from what java's
StrictMath.log10
computes. It's an engineering tradeoff which supports returning
an answer quickly, even if it isn't completely accurate.
You should decide which specification you choose to embrace.
automated test suite
System.out.println(log10(1000));
This is nice enough, I guess.
Certainly it exercises the target code,
and could reveal e.g. embarrassing divide-by-zero errors.
But it is not self-evaluating code.
It does not "know the answer".
It cannot display a Green bar,
nor break the build with a Red bar.
You don't have to use
JUnit
for automated unit tests.
But it's easy, so you may as well take advantage of what it offers.
I note in passing that there's no "negative x" test call.
You have the advantage of being able to call
the well-tested Math.log10() function in your tests,
so it's easy to draw input values from a wide range of numbers
and verify your function returns the correct result,
or in any event returns what the Math library returns,
which is pretty much the same thing.
algorithm
The Integer.toString() function contains some well-tested looping code,
and it offers a convenient .length() getter.
You could choose to use that as the core of a rather
different log10 implementation.
in what order should the assignments in the while and if block be written?
(Since both arrangements are possible without semantic change.)
I do not especially care;
it doesn't have a huge effect on readability.
while (x / 10 > 0) {
a++;
x /= 10;
}
I have a weak preference for reordering that so the a increment comes last.
The idea is to visually highlight that x divided by 10 appears twice.
But as mentioned above,
a less cryptic comparison would be nice,
in which case order wouldn't matter.
if (x1 > 10) {
x1 /= 10;
r += frac;
}
I really like that we mess with r last, here.
We focus on value of x1, comparing it and then altering it.
Nice.
It is unclear whether this code achieves its design goals.
This is due to the lack of several things:
- meaningful variable names
- meaningful helper function names
- cited reference
- /** javadoc */ spelling out
log10's contract
- automated unit tests
- absent the above,
// comments could be a poor substitute that partly makes up for a lack of above items
I would not be willing to delegate or accept maintenance tasks
on this codebase in its current form.