Skip to content
This repository has been archived by the owner on Mar 26, 2022. It is now read-only.

Rename constants for clarity #16

Closed
jonpemby opened this issue Apr 29, 2019 · 2 comments · Fixed by #19
Closed

Rename constants for clarity #16

jonpemby opened this issue Apr 29, 2019 · 2 comments · Fixed by #19

Comments

@jonpemby
Copy link
Contributor

jonpemby commented Apr 29, 2019

Hello! Great project!

Just looking through the code and I thought maybe it would make sense to rename SPELL_SET1 to SPELL_ADJECTIVES and perhaps SPELL_SET2 to SPELL_VERBS.

We could also remove the class constants in favor of using SPELL_ADJECTIVES.size in the range at https://github.com/K-Sato1995/spell_generator/blob/master/lib/spell_generator/generator.rb#L13 (for example).

Curious what your thoughts are on this.

@K-Sato1995
Copy link
Owner

Hi @jonpemby
Thanks for opening this issue!

it would make sense to rename SPELL_SET1 to SPELL_ADJECTIVES and perhaps SPELL_SET2 to SPELL_VERBS.

Definitely!! Those constants would be a lot more descriptive😄!!

We could also remove the class constants in favor of using SPELL_ADJECTIVES.size in the range

I totally agree with you here as well. That would make the code more intuitive and easier to read🙏!!

Are you interested in working on this??

@jonpemby
Copy link
Contributor Author

Sure @K-Sato1995! I opened a PR with my suggested changes. 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants