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

Finds the mean of a list of numbers #82

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Bigbublik
Copy link

No description provided.

@ZoomRmc
Copy link
Contributor

ZoomRmc commented Oct 5, 2024

Please, read this repo's contribution guidelines. If you had followed them, you'd know that your code doesn't work in its current form - the tests would show it.

  1. The file should include a module-level doc-comments, the minimal requirement is the module's name in the first line for auto populating the directory.
  2. Examples used in the documentation should be runnableExamples for additional testing. One example is enough, the rest should be tests.
  3. The module lacks tests. Follow the minimal example and merged algorithms on how to implement them.
  4. nums should be an openArray[SomeNumber]. You'll have to use a loop or std/sequtils.foldl, or import the std/math.sum which works on openArrays.

@dlesnoff
Copy link
Collaborator

dlesnoff commented Oct 7, 2024

Sums had a specific module in the stdlib, but were changed to a nimble package. See: nim-lang/Nim#18438
It did not have a mean function though.

Anyway, I would keep a similar version with openArray[SomeFloat] and maybe have a more complex version with openArray[SomeNumber]. I might only keep the second version but I would like to compare both.

I second @ZoomRmc : tests from the documentation could be changed copied under the when isMainModule.
Please, mark your PR as draft if you are not done. The following code indicates so:

when isMainModule:
  discard

Comment on lines +9 to +24
## ```nim
## echo mean(@[3, 6, 9, 12, 15, 18, 21])
## # Output: 12.0
## ```
## ```nim
## echo mean(@[5, 10, 15, 20, 25, 30, 35])
## # Output: 20.0
## ```
## ```nim
## echo mean(@[1, 2, 3, 4, 5, 6, 7, 8])
## # Output: 4.5
## ```
## ```nim
## echo mean(@[])
## # Error: List is empty
## ```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this under the when isMainModule block.

Copy link
Collaborator

@dlesnoff dlesnoff left a comment

Choose a reason for hiding this comment

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

See my answer in the conversation tab.

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.

3 participants