Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate character prevention bug #3

Merged
merged 3 commits into from
Jan 29, 2021

Conversation

gdebrauwer
Copy link
Member

The encoding process prevented duplicate characters in the output string. This meant it removed every used character from the character list. This causes certain numbers to have the same encoding result. This PR removes that logic, so each number is encoded correctly.

If you already use this package, this means every generated unique-code will change. By changing the encoding logic, the encoded result of each number will change, which can create collisions with older encoded numbers. You must change the length of all new codes, as this ensures every newly generated unique-code will never collide with older unique-codes.

@gdebrauwer gdebrauwer mentioned this pull request Jan 28, 2021
@gdebrauwer
Copy link
Member Author

We should probably tag this as a 2.0.0, otherwise, projects that already use the package might automatically upgrade.

@markvesterskov
Copy link

Just chiming in to be notified when 2.0 is tagged, using this for an upcoming app. Thanks for a useful package!

@lduhejian
Copy link

    protected function getMaximumUniqueCodes()
    {
        $maxCombinations = 1;

        for ($i = 0; $i < $this->length; $i++) {
            $maxCombinations = $maxCombinations * (strlen($this->characters) - $i);
        }

        return $maxCombinations;
    }

This function might also needs to be modified.

    protected function getMaximumUniqueCodes()
    {
        return pow(strlen($this->characters),$this->length);
    }

@gdebrauwer gdebrauwer merged commit cfab14b into master Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants