Skip to content

docs, test: genetic algorithm#338

Merged
siriak merged 1 commit into
TheAlgorithms:masterfrom
tikhoplav:gentic_algorith
Sep 6, 2021
Merged

docs, test: genetic algorithm#338
siriak merged 1 commit into
TheAlgorithms:masterfrom
tikhoplav:gentic_algorith

Conversation

@tikhoplav

Copy link
Copy Markdown
Contributor

Description of Change

Package genetic_algorithm/geneticalgorithm contains single function called GeneticString. Couple changes has been made to the code of the function related to providing configuration to function and retrieving result of the function execution.

Previously, function used to return tuple which contains result of generation as well as some details about generation process. Output was changed to the single go struct and an error, in order to make function testable (in case of errors).
In comparison with the previous output, when only a resulting string was provided, new output also contains a value, used inside the process, what can be useful for later function extensions.

Configuration previously hardcoded, now can be provided withing configuration structure. This feature can be used to optimize execution time. Despite the fact that PR does not provide any benchmarks, ability to provide configuration can be used to create a specter of tests, showing function efficiency based on the configuration parameters.


Checklist

  • Added description of change;
  • Added tests and example, test must pass;
  • Added documentation so that the program is self-explanatory and educational;
  • Relevant documentation/comments is changed or added;
  • PR title follows semantic (With exception of merging docs and test in one title, because otherwise PR makes no sense);
  • GitHub action golang_lint_and_test passed in fork;
  • Search previous suggestions before making a new one, as yours may be a duplicate;
  • I acknowledge that all my contributions will be made under the project's license.

Notes

Testing provided with this PR is nominal. Still more relevant tests could be added such as edge case testing and tests for correct error occurrences.

Previously description of the package stated using of concurrency. It was removed, because no actual concurrency has been used in the implementation.

@tjgurwara99 tjgurwara99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@tjgurwara99

Copy link
Copy Markdown
Member

@siriak could you review and merge this, I'm in the process of cleaning up the directory structure and this would create a merging conflict which I would need to solve before I progress with my PR further.

@siriak siriak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@siriak siriak merged commit c3de81f into TheAlgorithms:master Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants