Skip to content

Conversation

@manumafe98
Copy link
Contributor

@manumafe98 manumafe98 commented Apr 3, 2024

closes #150

I have a couple of doubts about this:

  • Naming of the new comment
  • If I should pass the list to the comment as a parameter, so we do not hardcode the methods in the comment of website-copy and instead use a variable (if possible, because I only use it with strings, so Im not sure if its possible)

Related to the second point, I guess I could create a method to format the elements of the list in a string that looks the likes of: "substring, split" but if we take that decision, we should extract that logic from the analyzer? so then we can reuse it with another analyzers in the future? or we dont mind repetition in this case

@manumafe98 manumafe98 added the x:size/small Small amount of work label Apr 3, 2024
@manumafe98 manumafe98 self-assigned this Apr 3, 2024
@manumafe98 manumafe98 requested a review from a team as a code owner April 3, 2024 19:41
@sanderploegsma
Copy link
Contributor

I'm wondering whether this approach makes it a bit too complex. Can't we decide to recommend using either substring or split in the comment (so not to mention both methods but one of the two, whichever we prefer) and update the analyzer implementation to follow these rules:

Scenario Analyzer output
Solution using substring N/A
Solution using split N/A
Solution using some other approach Use substring method*

* Or split if we think that's better than using substring

@manumafe98
Copy link
Contributor Author

I'm wondering whether this approach makes it a bit too complex. Can't we decide to recommend using either substring or split in the comment (so not to mention both methods but one of the two, whichever we prefer) and update the analyzer implementation to follow these rules:

Scenario Analyzer output
Solution using substring N/A
Solution using split N/A
Solution using some other approach Use substring method*

  • Or split if we think that's better than using substring

Sure, we can still use the same comment to recommend using substring then when we detect that neither split or substring has been used

@manumafe98 manumafe98 merged commit f5ba7ba into exercism:main Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:size/small Small amount of work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Log Levels: feedback incorrectly recommends to use the substring method

3 participants