Bug 62738 - RANDBETWEEN function rounds the value down to int
Summary: RANDBETWEEN function rounds the value down to int
Status: NEW
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: 4.0.0-FINAL
Hardware: PC All
: P2 major (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-19 11:52 UTC by Anton Egorov
Modified: 2018-10-03 17:03 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Egorov 2018-09-19 11:52:15 UTC
I have a cell in Excel file with the following formula:

=RANDBETWEEN(0, 9999999999)

When I call FormulaEvaluator::evaluateAll() the cell mostly gets the value 2147483647 (signed int32 max), which is not very random. The formula works fine in Excel.

I briefly looked at the code of org.apache.poi.ss.formula.atp.RandBetween class, and the int cast in the line 

return new NumberEval((bottom + (int)(Math.random() * ((top - bottom) + 1))));

seems to be the problem.
Comment 1 PJ Fanning 2018-09-19 12:25:05 UTC
I just changed the int cast to a long cast.
I'm sure there is a fair bit that could be done to improve the implementation and testing of this function. Formula Evaluation is not an area that has received much attention in recent times.

In the cases where users are reading in Excel files with POI, the Excel file has both the formula and an already calculated value - most users are happy with the pre-calculated value.
In the cases where users are writing Excel files with POI, they can add the formula cells and let users open the files in Excel and have it calculate the values.
There are some users who want POI to eval the formulas but we need more contributors to help with building up this code and its tests.
Comment 2 Yegor Kozlov 2018-10-03 17:03:45 UTC
I think we can return java.util.concurrent.ThreadLocalRandom.current().nextDouble(bottom, top + 1). It is the right way to generate a random number in a range in Java 1.7+.


Also, the code sets bottom to top if it is greater than it:

		if(bottom > top) {
			top = bottom;
		}

This check changes the semantics of RANDBETWEEN. Excel returns #NUM!  in such a case