It isn't complete yet and I have more stuff to add and some stuff still needs clarification, like Rule 4.3 (just created an issue for it).
If you have suggestions, feedback, things you disagree with, I'm happy to make edits or to accept issues and pull requests :-)
2.4 I would caveat that useless traits are useless in application code, but make sense in libraries published for use by other teams.
2.5 Disagree. The general contract of equals and hashCode does not demand that they are immutable; there are a wide variety of mutable java classes that offer similar equals and hashCode. Other aspects of the case class sugar can be very useful even for a mutable type. The correct solution is to avoid equals and hashCode in general (they're broken by default anyway); use something like scalaz's Equal, and don't define Equal instances for mutable types.
2.7 You should explain a bit about how to achieve the same results using e.g. scalaz.\/ and for/yield.
3.1 I find the arguments here very weak.
3.4 Stream has many problems and Iterable is hard to reason about. I think the suggestion to use lazy collections is bad advice. Temporary intermediate values ought to be caught by escape analysis and not cause GC pressure at all. I have never seen hard evidence of such values causing a problem in a real-world program. 3.3 applies.
4.2 It's worth mentioning the advantages of scalaz Future (more sensible handling of ExecutionContexts, also theoretically higher performance because of having fewer task boundaries).
Also I would emphasise that an actor with no mutable state is almost always a mistake (it should usually be replaced with use of Future or similar), as this is a common anti-pattern I see in newcomers' code.
4.3 It's reasonable to put actually heavy computation on dedicated threads (so that I/O threads can pre-empt compute threads). So there are times when this does make sense. You would certainly expect an explicit ExecutionContext in that case though.
5.2 Disagree. become is harder to reason about than ordinary mutable state (var and friends), as your example demonstrates. Actors encapsulate mutable state, that's their raison d'etre, if you're using them at all you've already given up on purity or referential transparency.
2.4. You missed my point. I didn't say traits are useless, but that people shouldn't define traits that are useless.
2.5. That rule includes a reference to a paper from 1993. And yes, the contract of equals and hashCode is broken. People should strive for sane equality, not a sane Object.equals, because there is not such thing. If that's not enough to convince you of it, because syntactic sugar, we'll have to agree to disagree but to be honest I refuse to work with code violating such a fundamental rule.
2.7. Thanks for the feedback, will do.
3.1. Point taken, will add more details.
3.4. What problems does Stream have? Also for me, those operators make much more sense in the context of lazy collections - that Scala's library does such a shitty job on lazy collections, that's a pet peeve of mine. On "escape analysis", I hope you're not relying on it because it doesn't work. The algorithm implemented by the JVM is very limited.
On evidence, I have, countless of times. But you know, search for the "mechanical sympathy" google group and ask there. Rule 3.3 applies to doing optimizations, but that doesn't mean one has a free pass to do stupid shit, ignoring CS 101. I personally don't see a conflict there.
4.2. I'm not familiar with Scalaz' Future. Will take a look.
On actors with no mutable state, I agree, but I also see another problem in that simple actors with mutable state could be modeled without state, as Futures (or similar) as well. I find it tough to explain to people when Futures should be used instead of Actors. I find it tough to make that choice myself. That's why this rule doesn't try to impose something specific - its purpose being to make people think about tradeoffs and not use something blindly.
4.3. This rule already has an issue started by me, as a reminder to myself that I did a shitty job - https://github.com/alexandru/scala-best-practices/issues/8
5.2. I think you miss-understood that rule. I'm not saying that actors should not mutate their state, but I'm saying that it is better to control the scope of those variables by means of context.become in combination with pure functions. You know, the Erlang way.
Also, actors are really NOT about mutating state willy-nilly, sorry but that misses the forrest from the trees. Actors are about bidirectional asynchronous communications, non-determinism and state machines. And as always, just because mutation happens, that doesn't mean you can't limit its effects. And it's actually really, really advised to do so on the JVM, in the presence of Futures that can capture the actor's internal state and so on and so forth.
2.4 No, you missed my point. Useless traits like the one in your example can be reasonable in the context of a library that needs to be able to evolve while maintaining backwards compatibility. And I think this distinction is worth drawing explicitly, because otherwise people will default to doing the "safe" thing everywhere, like they did with JavaBean getters/setters (which are a sensible if tedious thing for Java libraries to use, but pure wastefulness in application code).
2.5 Honestly if you're working with JVM libraries, you're already working with a lot of code that violates that "fundamental rule". And if you're not working with JVM libraries, why would you bother using Scala rather than Haskell? :P.
4.2/actors with no mutable state: It's a guideline that has plenty of false negatives - it doesn't capture all the cases where someone writes an Actor that ought to just be a Future or similar - but I don't think it has many false positives, and it's just such a common mistake that for me it's worth sharing explicitly.
5.2 I'm prepared to be convinced, but what you've written there doesn't convince me. I don't see any difference between the two examples in terms of being "pure, immutable or referentially transparent", and to my eyes the accidental complexity of "become" (a new concept) is higher than that of ordinary mutable variables. Telling me that that way is "the Erlang way" or to "just wait until you'll have to model a state machine with 10 states in it and dozens of possible transitions and effects to go along with it" sounds like bluster, tbh. Maybe it just needs better examples.
I seen this on Twitter and just by scanning it quickly I learned some things, so that's why I posted it here and thanks for sharing.
Can I ask about rule 5.2. SHOULD mutate state in actors only with "context.become"?
How would you use become to avoid using vars to keep state in an Actor like this?
class Account(var account: AccountDetails) extends Actor { def receive = { case msg: UpdateAccount => { account = msg.account } } }
However, 5.2 does not concern the elimination of vars, but rather better management of multiple states (Finite state machines)
Your example, just like the example in 5.2, only has a single state, so no context.become is strictly necessary. The author makes clear that when you have multiple states, the collection of top-level mutable vars will be hard to maintain. I had this problem a few times and I am glad this gets mentioned in the guide.
I agree though, that a better example would include at least two states.
Another side-effect of this for example is that you're not going to worry about vars that leak into Future computations, which is a mistake that even experienced developers are doing, because it's an easy one to make.
Here's your sample translated into a state-machine with 2 states: https://gist.github.com/alexandru/63eebe8e49e796f31e73
Thanks for the explanation and the code.
Quick comment on item 2.3: SHOULD NOT update a var using loops or conditions.
you conclude that this expression is the best option: val sum = elements.map(_.value).sum
This expression is not equivalent to the previous options in performance and memory, as the map expression will create a entire new sequence.
For large sequences, a better option is to write: val sum = elements.view.map(_.value).sum