3 ms·

I feel like I'd still want the body on its own line from the signature definition just for readability. And with the parser problems described in TFA, I'd say

by Pxtl 3y ago

I feel like I'd still want the body on its own line from the signature definition just for readability. And with the parser problems described in TFA, I'd say that parens should be mandatory coding-style enforced by linter. def initialize(text) = (@text = text) def inspect = ("#<#{self.class} #{text}>") def ==(other) = (other.is_a?(Word) && text == other.text) seems like a good compromise. edit: this is similar to our standard when writing one-liner methods in C#. A bit noisier because of types and visibility, but still pretty concise, imho. public void Initialize(string text) => Text = text; // I actually hate the above a bit because "=>" // originally implied functional/no-side-effects // in C#, but the world moves on. public string Inspect() => $"{this.GetType().Name} {text}>"; public static bool operator== (Word a, Word b) => a.Text == b.Text;

vidarh 3y ago
Personally I'd never let your first example through a code review. If you're going to break it up, use end. Where the one line format makes sense is when the body is trivial and often when there are multiple related methods that can be lined up to highlight their similarities and differences in the single line format.
- Pxtl 3y ago
  Hah, I'm a firm "no alignment allowed, indent only" reviewer, so that use case would never even occur to me.
  - vidarh 3y ago
    I can't stand that. It dramatically reduces code readability to me.
    - Pxtl 3y ago
      I tend to agree with Google on this one: https://google.github.io/styleguide/jsguide.html#formatting-horizontal-alignment https://google.github.io/styleguide/jsguide.html#formatting-... > Terminology Note: Horizontal alignment is the practice of adding a variable number of additional spaces in your code with the goal of making certain tokens appear directly below certain other tokens on previous lines. > > This practice is permitted, but it is generally discouraged by Google Style. It is not even required to maintain horizontal alignment in places where it was already used. > > Here is an example without alignment, followed by one with alignment. Both are allowed, but the latter is discouraged: { tiny: 42, // this is great longer: 435, // this too }; { tiny: 42, // permitted, but future edits longer: 435, // may leave it unaligned }; > Tip: Alignment can aid readability, but it creates problems for future maintenance. Consider a future change that needs to touch just one line. This change may leave the formerly-pleasing formatting mangled, and that is allowed. More often it prompts the coder (perhaps you) to adjust whitespace on nearby lines as well, possibly triggering a cascading series of reformattings. That one-line change now has a "blast radius." This can at worst result in pointless busywork, but at best it still corrupts version history information, slows down reviewers and exacerbates merge conflicts.

by Pxtl 3y ago

vidarh 3y ago

Personally I'd never let your first example through a code review. If you're going to break it up, use end. Where the one line format makes sense is when the body is trivial and often when there are multiple related methods that can be lined up to highlight their similarities and differences in the single line format.

Pxtl 3y ago
Hah, I'm a firm "no alignment allowed, indent only" reviewer, so that use case would never even occur to me.
- vidarh 3y ago
  I can't stand that. It dramatically reduces code readability to me.
  - Pxtl 3y ago
    I tend to agree with Google on this one: https://google.github.io/styleguide/jsguide.html#formatting-horizontal-alignment https://google.github.io/styleguide/jsguide.html#formatting-... > Terminology Note: Horizontal alignment is the practice of adding a variable number of additional spaces in your code with the goal of making certain tokens appear directly below certain other tokens on previous lines. > > This practice is permitted, but it is generally discouraged by Google Style. It is not even required to maintain horizontal alignment in places where it was already used. > > Here is an example without alignment, followed by one with alignment. Both are allowed, but the latter is discouraged: { tiny: 42, // this is great longer: 435, // this too }; { tiny: 42, // permitted, but future edits longer: 435, // may leave it unaligned }; > Tip: Alignment can aid readability, but it creates problems for future maintenance. Consider a future change that needs to touch just one line. This change may leave the formerly-pleasing formatting mangled, and that is allowed. More often it prompts the coder (perhaps you) to adjust whitespace on nearby lines as well, possibly triggering a cascading series of reformattings. That one-line change now has a "blast radius." This can at worst result in pointless busywork, but at best it still corrupts version history information, slows down reviewers and exacerbates merge conflicts.