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

Add a common check for boolean function naming patterns #180

Merged
merged 7 commits into from Oct 6, 2021

Conversation

jiegillet
Copy link
Contributor

@jiegillet jiegillet commented Oct 2, 2021

Closes #168.

This one is fairly straightforward.

@jiegillet jiegillet added x:module/analyzer Work on Analyzers x:type/coding Write code that is not student-facing content (e.g. test-runners, generators, but not exercises) x:size/large Large amount of work labels Oct 2, 2021
end

defp render_names(type, name) do
is_encloded_? = String.starts_with?(name, "is_") and String.ends_with?(name, "?")
Copy link
Contributor

Choose a reason for hiding this comment

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

Encloded - is that a word I don't know yet or a typo?

Copy link
Contributor Author

@jiegillet jiegillet Oct 3, 2021

Choose a reason for hiding this comment

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

Oh yeah, it's totally a new word that the cool people know about, but for noobs, I guess I should change it to is_enclosed_? or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a noob, I am thankful for the change 🙏 After turning 25 I stopped being able to catch up with what's cool now and what isn't. I still try to spell words correctly, like a nerd 😞

true -> "#{name_without_?} or #{type} #{name_without_is}"
end

{"#{type} #{name}", "#{type} #{correct_name}"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering about the comment here. In case of defmacro is_foo?, we will have those variables.

actual = "defmacro is_foo?"
expected = "defmacro is_foo or defmacro foo?"

Now we have no way of enclosing both potential names separately with backticks in the markdown comment because they're in a single variable. The best we can have is something like this, which isn't technically correct Elixir code:

expected:

defmacro is_foo or defmacro foo?

I'm thinking maybe we need three different comments. One for functions and one for guards, both of which have a single expected name, and a third one for macros, that will have two expected names. This would also allow us to write more tailored explanations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One way to make it correct code is to change to # or defmacro foo?, but your suggestion is better :)

Copy link
Contributor

@angelikatyborska angelikatyborska left a comment

Choose a reason for hiding this comment

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

Let's goooo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:module/analyzer Work on Analyzers x:size/large Large amount of work x:type/coding Write code that is not student-facing content (e.g. test-runners, generators, but not exercises)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a common check for is_foo? function names that mix up two separate naming patterns
2 participants