Repository navigation
Use named export for largest series product - #556
Closed
serixscorpio wants to merge 5 commits into
Closed
serixscorpio wants to merge 5 commits into
serixscorpio wants to merge 5 commits into
Conversation
Per #436, convert default export to named export for largest series product exercise. Change from class style `new Series(x).largestProduct(y)` to `largestSeriesProduct(x, y)` since there is no need to keep state.
This refactor is to validate input as early as possible.
Instead of `forEach`, use `reduce` to convey clearly `max` as an accumulator. Also, `product` only need to be within the reducing function.
Member
|
I think it would be better if you move the named export changes to another PR and leave this one for eslint only. |
Contributor
|
@serixscorpio we appreciate your contributions, truly! But I am in agreement with @tejasbubane that the PRs should be logically separate. Just makes it easier for the easily confused among us to follow the plot :) |
Contributor
Author
|
Thanks for your feedback, I'll open a separate PR to lint first, and another one to move to named export (in the order suggested) to reduce the cognitive load. Thanks. |
Contributor
Author
|
closing this in favor of alternate PR #561, which does the named export after linting |
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.
Per #436, convert default export to named export for largest series
product exercise. Change from class style
new Series(x).largestProduct(y)to
largestSeriesProduct(x, y)since there is no need to keep state. See 8a38f2f.As part of this change, I also simplified and linted the pre-existing logic in the example solution (
c2d080c, e9e737b, c450f53 and 40504cf. If these are too noisy to be within the same PR, please let me know. I can break these into a separate PR. Thanks!