Bugfix and improvements to JS version#4
Open
RedPhoenixQ wants to merge 17 commits intoLenaSYS:masterfrom
Open
Conversation
getRandomInt excludes the max value and the list cannot be indexed by the length, so one should not be added to length. This caused some "crashes" where undefined was returned.
This avoids an unnecessary division and extra calculation when creating the random number. The output will still be a number between 0 and 1.
This makes the checks for plurality later in the code much clearer and easier to read.
This makes the code shorter and clearer as no specific documentation is needed on a another function.
This allows for two helper functions to be removed and for the code to be clearer and more efficient. The string is iterated through fewer times as the number of replacements does not need to be found before performing the replacement. Previously this was atleast two linear searches for each replacement.
substr is not enforced by the javascript standard and might not exits in all environments
The min value is never used and can be trivially recreated externally. This removes some repeated unneseccary operation.
Using JSDoc allows for improved integration when writing code with an LSP. It is still readable for humans and combines the best of both worlds
This will allow for clearer variable names in the rest of the code
This includes simplifying branches and modifying logic to make code smaller and more efficient. There is no longer a need for the step to remove staring space and double spaces as they are no longer generated.
This makes for fewer functions which makes it easier to copy if needed
This is clearer and get directly parsed as an array and is not an expression that needs to be evaluated
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This firstly fixes a bug were
getRandomIntwould get an item outside outside the array, leading to an undefined being used later.The output given the same random number will not be the same because if this bugfix, so the rest of the changes does not aim to generate the same output as before. Additionally an empty string was removed from one of the arrays that would previously cause missing words in sentences.
Most helper functions have been changed to use builtin functions like
replaceAllinstead. Many redundant operations have also been removed so the code is as simplified as possible.The functions have also been given JSDoc comment which integrates with code editors to give type hints while writing code. The descriptions of what the function does and how the parameters work and interact have also been improved.
This does result in a more than 2x speed improvement while almost cutting the number of line of code in half.