Style Question

73 views
Skip to first unread message

Eli Barzilay

unread,
Aug 29, 2016, 6:29:59 PM8/29/16
to scala...@googlegroups.com
Pre-apologies for a question that is probably well-hashed, but there's
an issue that bugs me every time I run into it: how to deal with long
lines. To put things in context, in the coursera class I have this gem
in my code:

...
case Fork(left, right, chs, _) =>
convert(left ).map(k => (k._1, 0 :: k._2))
++ convert(right).map(k => (k._1, 1 :: k._2))
...

which runs into an error about having no value for ++. AFAICT, my only
options for fixing this expression are:

* Have it formatted with the ++ on the first line. I dislike that since
the outer operation (++) is pushed to the right, where it can easily
be hidden when lines have different widths.

* Wrap it (the ++-expression) in parens, either ()s hugging the code, or
{}s with a block-like look. I dislike the idea of using parens just
for line wrapping.

* Go with the conventional quasi-wisdom of "the expressions are long
enough, so introduce intermediate values" -- not only do I not agree
with that sentiment as a general rule (example in the above: long, but
simple), but doing that would require adding {}s which goes back to
the previous point.

So: is there no other way around this?

[I know that style questions can flame quickly, but that's not my
intention: all of these solutions are things that I don't like to the
point that I'd be happy to use some backslash at the end of the line...]

--
((x=>x(x))(x=>x(x))) Eli Barzilay:
http://barzilay.org/ Maze is Life!

Kevin Wright

unread,
Aug 29, 2016, 6:47:08 PM8/29/16
to Eli Barzilay, scala-user
Actually… You *don’t* need to add a wrapping block inside a case condition. So this is perfectly valid:

case Fork(left, right, chs, _) =>
  val lefts = convert(left) map { case (k, v) => (k, 0 :: v) }
  val rights = convert(right) map { case (k, v) => (k, 1 :: v) }
  lefts ++ rights
case SomethingElse =>


You’ve also got enough similarity there that it could be factored out entirely:

def betterConvert(side: Type, prefix: Int) =
  convert(side) map { case (k, v) => (k, prefix :: v) }


case Fork(left, right, chs, _) =>
  betterConvert(left, 0) ++ betterConvert(right, 1)
case SomethingElse =>


    ...

--
You received this message because you are subscribed to the Google Groups "scala-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Kevin Wright
mail / hangouts / msn : kev.lee...@gmail.com
vibe / skype: kev.lee.wright

"My point today is that, if we wish to count lines of code, we should not regard them as "lines produced" but as "lines spent": the current conventional wisdom is so foolish as to book that count on the wrong side of the ledger" ~ Dijkstra

Eli Barzilay

unread,
Aug 31, 2016, 12:40:45 PM8/31/16
to Kevin Wright, scala-user
On Mon, Aug 29, 2016 at 6:46 PM, Kevin Wright <kev.lee...@gmail.com> wrote:
> Actually… You *don’t* need to add a wrapping block inside a case condition.
> So this is perfectly valid:
>
> case Fork(left, right, chs, _) =>
> val lefts = convert(left) map { case (k, v) => (k, 0 :: v) }
> val rights = convert(right) map { case (k, v) => (k, 1 :: v) }
> lefts ++ rights
> case SomethingElse =>

So the discount that I didn't know about here is that I don't need
braces, but the main problem I have with it is the introduction of
binders just because of line wrapping. (Specifically, the way that I
wrote it makes it perfectly clear that there's a left + right sides, and
that they're very similar, so adding bindings doesn't make it clearer
IMO.)


> You’ve also got enough similarity there that it could be factored out
> entirely:
>
> def betterConvert(side: Type, prefix: Int) =
> convert(side) map { case (k, v) => (k, prefix :: v) }

This is also what I referred to as quasi-wisdom (which in this case
(IMO!) results in code that is harder to read since you need to mentally
store another label for `betterConvert` which is used in just one
place).

Thanks anyway though!


[And BTW:

> "My point today is that, if we wish to count lines of code, we should
> not regard them as "lines produced" but as "lines spent": the current
> conventional wisdom is so foolish as to book that count on the wrong
> side of the ledger" ~ Dijkstra

that its exacly the reason I don't like that quasi-wisdom...]

Rex Kerr

unread,
Aug 31, 2016, 5:31:33 PM8/31/16
to Eli Barzilay, scala-user
On Mon, Aug 29, 2016 at 2:41 AM, Eli Barzilay <e...@barzilay.org> wrote:
Pre-apologies for a question that is probably well-hashed, but there's
an issue that bugs me every time I run into it: how to deal with long
lines.  To put things in context, in the coursera class I have this gem
in my code:

    ...
    case Fork(left, right, chs, _) =>
         convert(left ).map(k => (k._1, 0 :: k._2))
      ++ convert(right).map(k => (k._1, 1 :: k._2))
    ...

which runs into an error about having no value for ++.  AFAICT, my only
options for fixing this expression are:

* Have it formatted with the ++ on the first line.  I dislike that since
  the outer operation (++) is pushed to the right, where it can easily
  be hidden when lines have different widths.

In practice I don't find this to be much of an issue, for two reasons.
  (1) I make my window wider and/or have my editor wrap lines in display.
  (2) I know what should return a value and what should have a side-effect so I go looking for the operation

If the operations are _not_ all the same I'll introduce temporary variables and/or parentheses which are not strictly needed just to keep it very clear what the intent is.  But if it's a single operation and/or repeated identical operations, I think it's likely to be not-very-hard to train yourself (and for others to train themselves) a little to make this not an issue at all.

That the different statements end up better-aligned with this approach is, I think, a significant advantage too.  I much more often want expressions to be parallel in some way but mess it up than I mess up the operation on the end, so having the similarities and differences aligned and thus easier to spot is a bigger help than the drifting away of the operator is a loss.  If the operator is _really_ the thing, then you're best off introducing temporary variables anyway, so the gigantic expressions which are arguments to the operations don't obscure the key thing you care about, namely the operations.

  --Rex

Kevin Wright

unread,
Aug 31, 2016, 5:58:18 PM8/31/16
to Eli Barzilay, scala-user
> def betterConvert(side: Type, prefix: Int) =
>   convert(side) map { case (k, v) => (k, prefix :: v) }
 
This is also what I referred to as quasi-wisdom (which in this case
(IMO!) results in code that is harder to read since you need to mentally
store another label for `betterConvert` which is used in just one
place).

In practice, that method would be a one-liner and - if only used in this one place - would be defined as close as possible to the use site (remember, we can nest function definitions in Scala).

So no need to memorise anything, you can see it defined right there on your screen, within a scant few lines of where it’s used, in the extreme case:

case Fork(left, right, _, _) =>

  def betterConvert(side: Type, prefix: Int) = convert(side) map { case (k, v) => (k, prefix :: v) }
  betterConvert(left, 0) ++ betterConvert(right, 1)
case SomethingElse =>


Eli Barzilay

unread,
Aug 31, 2016, 9:36:03 PM8/31/16
to Kevin Wright, Rex Kerr, scala-user
On Wed, Aug 31, 2016 at 5:31 PM, Rex Kerr <ich...@gmail.com> wrote:
>
> In practice I don't find this to be much of an issue, for two reasons.
> (1) I make my window wider and/or have my editor wrap lines in
> display.

What I meant is that in most cases the RHS is not aligned, so the
operator gets lost in the noise.

> (2) I know what should return a value and what should have a side-effect
> so I go looking for the operation

That doesn't help when I read other people's code, or write code that
others should read.


> That the different statements end up better-aligned with this approach
> is, I think, a significant advantage too.

I'm guessing that you're talking about similar lines lining up since
there's no extra operator on the second line as in:

blah blah
+ blah blah

That's true, but there are indentations that can still make them line up
(see my example), and of course there's alternative syntaxes (as in Lisp
sexprs).


On Wed, Aug 31, 2016 at 5:57 PM, Kevin Wright <kev.lee...@gmail.com> wrote:
>
> In practice, that method would be a one-liner and - if only used in this one
> place - would be defined as close as possible to the use site

Not really... See how the extra syntactic baggage in your example
(function name, argument names, types) are roughly the same size as the
actual body.

> (remember, we can nest function definitions in Scala).

(Yes; I'm new to Scala, but I'm extremely familiar with FP/PL/Lexical
Scope.)


> case Fork(left, right, _, _) =>
> def betterConvert(side: Type, prefix: Int) = convert(side) map { case (k,
> v) => (k, prefix :: v) }
> betterConvert(left, 0) ++ betterConvert(right, 1)
> case SomethingElse =>

And indeed the result is (again -- IMO) much harder to read.

(Abstraction of code that is used only twice can be a problem that leads
you down to omega and from there to the Y combinator... See my
signature.)

In any case, I'll stop replying on the list now, sorry for the noise.

Lanny Ripple

unread,
Sep 3, 2016, 3:44:52 PM9/3/16
to scala-user, kev.lee...@gmail.com
Dijkstra wasn't into code golf as you can see in his collected notes. -- https://www.cs.utexas.edu/users/EWD/.  He was commenting against the idea that people be *measured* on LOC.  But don't take my word for it...

The practice is pervaded by the reassuring illusion that programs are just devices like any others, the only difference admitted being that their manufacture might require a new type of craftsmen, viz. programmers. From there it is only a small step to measuring "programmer productivity" in terms of "number of lines of code produced per month". This is a very costly measuring unit because it encourages the writing of insipid code, but today I am less interested in how foolish a unit it is from even a pure business point of view. My point today is that, if we wish to count lines of code, we should not regard them as "lines produced" but as "lines spent": the current conventional wisdom is so foolish as to book that count on the wrong side of the ledger. - https://www.cs.utexas.edu/users/EWD/transcriptions/EWD10xx/EWD1036.html

If a construct is helpful and increases the manageability of the code then it's LOC well spent.

$0.02,
  -ljr

Russ P.

unread,
Sep 7, 2016, 1:58:02 AM9/7/16
to scala-user


On Monday, August 29, 2016 at 3:29:59 PM UTC-7, Eli Barzilay wrote:
Pre-apologies for a question that is probably well-hashed, but there's
an issue that bugs me every time I run into it: how to deal with long
lines.  To put things in context, in the coursera class I have this gem
in my code:

    ...
    case Fork(left, right, chs, _) =>
         convert(left ).map(k => (k._1, 0 :: k._2))
      ++ convert(right).map(k => (k._1, 1 :: k._2))
    ...

which runs into an error about having no value for ++.  AFAICT, my only
options for fixing this expression are:

* Have it formatted with the ++ on the first line.  I dislike that since
  the outer operation (++) is pushed to the right, where it can easily
  be hidden when lines have different widths.

I may be missing something, but this seems like the obvious answer to me. If you are going to split an expression, always put the operator at the end of the line, NOT at the beginning of the next line. I thought that concept was well established. An aesthetic side benefit is that the two similar lines will line up nicely!
Reply all
Reply to author
Forward
0 new messages