search

ec4j / editorconfig-maven-plugin

A Maven plugin for checking whether project files comply with format rules defined in .editorconfig files and eventually also for fixing the violations
Apache License 2.0
54 stars 13 forks source link

Refactor some classes #63

Closed bennetelli closed 3 years ago

bennetelli commented 3 years ago
ppalaga commented 3 years ago
  • I renamed AbstractEditorconfigMojo to AbstractEditorConfigMojo with an upper C to be consistent to the rest of the codebase.

+1 for the consistency

  • Also rename CheckMojo and FormatMojo to EditorConfigCheckMojo and EditorConfigFormatMojo

I see what you do, but it would be nice to know why you think that longer names are better?

  • The other refactoring replaces a switch case with an if statement which improves the readability of the code

Ok, thanks, readability is a valid reason. The original might be more performant for this enum with five items thanks to being compiled using a tableswitch instruction.

bennetelli commented 3 years ago

"I see what you do, but it would be nice to know why you think that longer names are better?"

IMO it should be consistent as well. Both classes are extending AbstractEditorConfigMojo so they should have a name which shows these classes belong to each other even when you don't know the content of these classes.

bennetelli commented 3 years ago

"Ok, thanks, readability is a valid reason. The original might be more performant for this enum with five items thanks to being compiled using a tableswitch instruction."

Is a switch really more performant? I actually never thought about it. Then we could also leave it as it is right now.

ppalaga commented 3 years ago

Is a switch really more performant? I actually never thought about it. Then we could also leave it as it is right now.

Yes, I'd say so. It is less instructions for the second and all following items.

bennetelli commented 3 years ago

Just reverted the switch/if statement changes.

ppalaga commented 3 years ago

Thanks for the contribution and thanks for the patience with the review!

bennetelli commented 3 years ago

You're welcome! And thanks to you as well :)