Plugin Classes gain name of constructor #485
Conversation
| return true; | ||
| } | ||
| if (!pluginCounter) pluginCounter = 0; | ||
| if (!job) return true; |
There was a problem hiding this comment.
my coding style is never to do such "oneliners". always use braces for code blocks.
it keeps a consistent style, avoids bugs.
if you feel like the optimization makes code shorter, perhaps the function is too long, has too many responsibilities, and should be refactored by method extraction, etc. removing braces, does not make the code simpler.
also, for code minifying there's automated tools, no need to obfuscate code manually.
my 2ç
There was a problem hiding this comment.
Thanks for your feedback!
I generally disagree, coming from a Ruby background. In my experience, folks tend to be able to be able to parse the logic of one liners more easily, as long as they are simple (return or set a value). The bounds of this conversation and the complexity of what would be allowed in 1-liners is checked be Prettier (Linter), which is comfortable with "short" 1-liners.
There was a problem hiding this comment.
Spotted micro optimization, these lines should be swapped:
if (!pluginCounter) pluginCounter = 0;
if (!job) return true;there's no point sanitize pluginCounter if next line returns from the method.
There was a problem hiding this comment.
Exactly the kind of thing that's easier to see with these one-liners 👍
Lines swapped @ 664b58d
There was a problem hiding this comment.
thought to share this: https://blog.codecentric.de/en/2014/02/curly-braces/
This PR changes the
this.nameattribute of plugin classes to match the name of the class. For example, nownew JobLock()will havethis.name = 'JobLock'.While possibly useful in debugging, this is important to un-block #479, which currently will consider each instance of a plugin the same as the base class.