Refactoring misc (add override modifier, return types explicitly and so on)

[zaneli]

I think these are better coding style, but it might be my personal preference. So I'd like to get maintainers opinion.

[alexandru]

@zaneli, I appreciate your contribution, but why would you want to add an explicit override?

I mean, I understand the utility on overriding an existing method, but on implementing an interface it's not that useful and it's just ceremony imho.

What do you say?

[zaneli]

@alexandru

why would you want to add an explicit override?

Please refer to the databricks/scala-style-guide. I agree this in principle.

e.g. These cases should be compile error.

class Foo(val name: String) {
  // I want to override java.lang.Object.equals, but I have mistaken the argument's type...
  def equals(o: Foo): Boolean = {
    name == o.name
  }
}

When write override explicitly, method equals overrides nothing. compile error occurs. (Of couse, I understand I should be use case class instead of to overriding equals in this case, but I guess there are other similar case.)

But I don't have a strong preference for it. It does not matter even if I'll revert or be rejected it.

[lloydmeta]

I disagree with using override when there is no actual overriding happening (e.g. implementing an abstract method). It is obscuring the meaning of a comsci word to fit the convention that Java people have used it for. The first thing I think about is "what is the original implementation" when I see the word override. Anyways, if you forget to implement an abstract method or implement the wrong method signature, your code will not compile.

I think in most cases, it will cause more headaches than it saves (e.g. imagine in the future you put a concrete implementation in a base class/trait -> if you had added override everywhere you would never get any errors).

For what it's worth, Odersky does not use override when implementing an abstract method.

[alexandru]

@zaneli that's a good point, but it's applicable to overridden methods. For abstract methods it has no utility, as if you get the signature wrong, the compiler will complain anyway. Plus I think overriding methods is sort of an anti-pattern and personally I prefer my classes to be final (though thankfully SpyMemcached doesn't do that) :-)

BTW, that style guide from databricks has really weird things in it that should not be in a Scala style guide. It's understandable for them to be conservative and performance oriented, since they are developing Apache Spark, but advising people that return is OK in some instances and being against using call by name, it's as if they are not using Scala :-)

Oh, but that sealed trait fix should go in. And I saw some explicit return types being added and for public stuff that's cool (public methods should have explicit return types). And I also saw some strings being reformatted. Can you make another PR without the overrides?

[alexandru]

coverage/coveralls — Coverage decreased (-0.7%) to 82.781%

Hahaha, see, you've added some keywords and now code coverage decreased. So if you add ceremony, you have to compensate with some extra tests. This is so cool :-)

[zaneli]

@alexandru @lloydmeta Oh, I see. Thank you!

Can you make another PR without the overrides?

Sure thing! I'll do it soon.