5 ms·

Hello, I'm the author of this list - I compiled it primarily for my colleagues, but hopefully it will help others. It isn't complete yet and I have more stuff

Hello, I'm the author of this list - I compiled it primarily for my colleagues, but hopefully it will help others. 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 :-)

century19 12y ago
Hi bad_user, 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 } } }
- wernerb 12y ago
  Actors are good at encapsulating state, therefore using mutable variales inside an actor through is very natural, and one might even question if an actor is needed if it does not have any mutable state inside it. 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.
  - bad_user 12y ago
    I'll add a better example, thanks for the feedback.
- bad_user 12y ago
  As @wernerb already pointed out, the idea is to use context.become such that inside your actor the only side-effects happening are due to context.become, otherwise the data structures are left immutable, the helper functions are left pure, etc... and it becomes easier (less error prone) to manage complex actors that way. 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 https://gist.github.com/alexandru/63eebe8e49e796f31e73
  - century19 12y ago
    OK, I think I get it. Will have to try out the example. Thanks for the explanation and the code.
netcraft 12y ago
Just as a matter of style I would put a comment like //dont do this or //bad practice in the negative examples, so that it stands out better.
- bad_user 12y ago
  Thanks for the feedback, will do :-) https://github.com/alexandru/scala-best-practices/issues/14 https://github.com/alexandru/scala-best-practices/issues/14
lmm 12y ago
1.2 I disagree with your stance (and your misuse of "NP-complete" in justifying it is a pet peeve; it is a technical term that has a specific meaning). IMO manually maintained formatting is not worth the cost; being able to run an autoformatter whenever code is unclear is a huge benefit. 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.
- bad_user 12y ago
  1.2. I do not agree and I tried justifying that with samples. If you want to use auto-formatters, that's fine, but I find it disrespectful to reformat the code of other people that know what they are doing. 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 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.
orp 12y ago
Hi, 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

century19 12y ago

Hi bad_user, 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 } } }

wernerb 12y ago
Actors are good at encapsulating state, therefore using mutable variales inside an actor through is very natural, and one might even question if an actor is needed if it does not have any mutable state inside it. 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.
- bad_user 12y ago
  I'll add a better example, thanks for the feedback.
bad_user 12y ago
As @wernerb already pointed out, the idea is to use context.become such that inside your actor the only side-effects happening are due to context.become, otherwise the data structures are left immutable, the helper functions are left pure, etc... and it becomes easier (less error prone) to manage complex actors that way. 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 https://gist.github.com/alexandru/63eebe8e49e796f31e73
- century19 12y ago
  OK, I think I get it. Will have to try out the example. Thanks for the explanation and the code.

netcraft 12y ago

Just as a matter of style I would put a comment like //dont do this or //bad practice in the negative examples, so that it stands out better.

bad_user 12y ago
Thanks for the feedback, will do :-) https://github.com/alexandru/scala-best-practices/issues/14 https://github.com/alexandru/scala-best-practices/issues/14

lmm 12y ago

1.2 I disagree with your stance (and your misuse of "NP-complete" in justifying it is a pet peeve; it is a technical term that has a specific meaning). IMO manually maintained formatting is not worth the cost; being able to run an autoformatter whenever code is unclear is a huge benefit. 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.

bad_user 12y ago
1.2. I do not agree and I tried justifying that with samples. If you want to use auto-formatters, that's fine, but I find it disrespectful to reformat the code of other people that know what they are doing. 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 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.

orp 12y ago

Hi, 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