Improve big integer number formatting

[antonkomarev]

• Follow up Ability to provide a base number to be added to the counter #74

[antonkomarev]

I've removed floats and wrote own string number formatter, because float numbers are formatting incorrectly anyway.

[antonkomarev]

@Brikaa Here are some things I changed before the release.

[Brikaa]

With this check, we currently have two sources of truth for checking whether the maximum number of views has been reached: the is_int check here and the assert less than MAX_COUNT assertion in the constructor. We know this wouldn't cause logical problems since MAX_COUNT will always be <= PHP_INT_MAX, but maybe it would be better if they become a single source of truth instead?

If we are sure that MAX_COUNT is always going to be PHP_INT_MAX and won't change in the future, then maybe we can remove the assertion in the constructor (along with MAX_COUNT) since it only receives ints and compares them against PHP_INT_MAX (ints will never exceed PHP_INT_MAX).

If MAX_COUNT might change in the future to be less than PHP_INT_MAX, then maybe we should find a way to do the check in one place or to do the same check here and in the constructor.

With the implementation in the PR, if MAX_COUNT changes, the message "the maximum number of views has been reached" in the plus function implies a different meaning than the same message in the constructor (albeit that doesn't cause any logical problems).

[Brikaa]

If MAX_COUNT might change in the future to be less than PHP_INT_MAX, then maybe we should find a way to do the check in one place or to do the same check here and in the constructor.

Or maybe, in that case, we can change the error message in the plus function to something along the lines of: "the number of views have exceeded the integer limit". This way we wouldn't have one error message having two different meanings (exceeding the integer limit and exceeding MAX_COUNT).

[antonkomarev]

Yeah this kinda bad design for now, but in reality I'm not sure we will ever reach the max value 😅

[antonkomarev]

It's just a protection vs type error in cases when people will try to send too big base value.

[antonkomarev]

Right now there are a lot of fraud requests from users, trying to avoid error log from the spam 😞

[antonkomarev]

I'd suggest to push it as is and rethink it later if someone wants to invest time into it .

[Brikaa]

Cool

[antonkomarev]

@Brikaa I know how we could handle enormous big integer. We can use brick/math package.

BigInteger::of('9999999999999999999999999999999999999999999');

But it will require to change database schema to store count as string instead of int.

Anyway we should limit max base_count value to not stuck with enormously huge numbers.

[Brikaa]

@antonkomarev maybe we don't need to change the schema (or to use BigInt). We can just avoid an int overflow in the file repository and the db repository. For the Count plus, we can either avoid overflowing (as with the current implementation) or use BigInt.