Re: [PATCH] greybus: Fix coding stye error

Johan Hovold
2017-01-27T14:01:28Z

On Fri, Jan 27, 2017 at 04:24:47PM +0300, Dan Carpenter wrote:
> On Fri, Jan 27, 2017 at 02:08:06PM +0100, Johan Hovold wrote:
> > On Fri, Jan 27, 2017 at 03:47:21PM +0300, Dan Carpenter wrote:
> > > The name sucks is what I'm saying. Please fix it eventually.
> >
> > I disagree. The name uses a common prefix that reflects the object it is
> > working on. This should not be changed. As it is currently named, the
> > function is also self-documenting. Trying to abbreviate the name just to
> > meet a 80 col limit (when there are alternatives) is just silly.
>
> gb_interface_request_mode_switch()
>
> It's not self documenting because there are no verbs in that sentence.

"request" is a verb.

We're requesting that the remote interface initiates a mode switch
(something which it can refuse).

It's a function that is only a called a couple of times. No need to
rewrite it as gb_intf_rms() or such (even if gb_intf would be a
reasonable prefix).

You comments about missing verbs etc make me wonder how you parse the
name:

gb_interface - a common prefix, indicating object
being acted on
request_mode_switch - describes what is done

we're not fetching anything as your "gb_get_mode_switch()" suggestion
indicates you read it as.

> I guess in JAVA it would be considered good style.
>
> This is "Enterprise Quality" in the derogatory sense of the term. The
> prefix makes everything too long and doesn't add any value. We don't
> keep the 80 character limit because of small screens, we keep it to
> discourage this type of code.

And to keep indentation down.

The kernel already has similar function names such as
"usb_driver_set_configuration" or "pm_runtime_mark_last_busy" which
follow the same pattern (prefix - verb - object).

Johan