5
\$\begingroup\$

I'm implementing a plus method that should work this way:

1 plus 2      // 3
"a" plus "b"  // "a and b = ab"

Here is my implementation:

object MyMath {
  implicit class MyMath(a: Any) {
    def plus(b: Any) = {
      (a, b) match {
        case (s: String, s2: String) => s"$s and $s2"
        case(i: Int, i2: Int) => i + i2
      }
    }
  }

  def main(args: Array[String]) {
    println(1 plus 2)
    println("a" plus "b")
  }
}

Should I avoid using type Any? Is this a sign of a code smell if I use Any too much?

\$\endgroup\$
2
  • \$\begingroup\$ Oh, you're using string interpolation, introduced in Scala 2.10.0. \$\endgroup\$ Jun 26, 2015 at 22:40
  • \$\begingroup\$ Yes, I'm sorry I haven't specify the version but the string interpolation is not important here. That can be replaced by: s1 + " and " + s2 \$\endgroup\$
    – HeeL
    Jun 26, 2015 at 22:44

3 Answers 3

10
\$\begingroup\$

First off: Apologies for the shortness of this review. There's not much code, after all.

You've got a bit of inconsistent style with whitespace after your cases -- I haven't done Scala in a while, but if I'm remembering right, you're supposed to have a space after.

I'd recommend renaming i and s to i1 and s1, respectively, just to be consistent.

Your 'spec' says that "a" plus "b" should return "a and b = ab", but it actually returns "a and b". Personally, I think it should return "ab", a la string concatenation.

Using Any is not a code smell, though some error throwing may be in order if any types other than Int or String are used, so that if I call 1.0 plus "a", I don't just get nothing. It's up to you which type to use, though personally I'd use an IllegalArgumentException.

Aside from that, it looks good! Well done.

\$\endgroup\$
3
\$\begingroup\$

This code will throw if you supply arguments of type that is different than any of case branches. If you use match there always has to be a branch to catch everything (or branches has to cover any possible type), much like switch { default: } in C++. It's not required by language (though scalac will produce warning), but sort of smelly code.

\$\endgroup\$
1
\$\begingroup\$

this is a possible implementation without Any ,I hope it helpful

trait Plus[T]{

  def plus(el :T):T

}

object Plus{

  implicit class PlusInt(d:Int) extends  Plus[Int] {
    override def plus(el: Int): Int = d + el
  }

  implicit class PlusString(d:String) extends  Plus[String] {
    override def plus(el: String): String = d + el
  }

}

this is the test

class PlusTest extends FunSuite  {

  import Plus._

  test("test") {

    assert(2 === (1 plus 1))
    assert("ab" === ("a" plus "b"))
  }

}

This solution in my opinion improves the other one because, if you don't implement for a class, the code will not compile, instead of throwing a runtime exception (MatchError).

\$\endgroup\$
1
  • \$\begingroup\$ Reviews should always include explanation, not simply an alternate implementation. One of the main points of this site is learning both how and why to do things "better". \$\endgroup\$ Oct 29, 2015 at 12:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.