MarsRover in Scala

This is my first attempt at Scala programming. I tried to be functional but I'm not sure if I have achieved that.

DataTypes.scala

object DataTypes {

  case class Position(xPos: Int, yPos: Int)

  sealed trait Direction

  case object N extends Direction

  case object S extends Direction

  case object E extends Direction

  case object W extends Direction

  case class RoverState(direction: Direction, position: Position)

  val leftRotation = Map[Direction, Direction](N -> W, W -> S, S -> E, E -> N)
  val rightRotation = Map[Direction, Direction](N -> E, W -> N, S -> W, E -> S)

}

RoverMachine.scala:

import DataTypes._

object RoverMachine {

  def initialRoverState(direction: Direction, position: Position): RoverState = {
    RoverState(direction, position)
  }

  def move(roverState: RoverState): RoverState = {
    roverState.direction match {
      case N => RoverState(N, Position(roverState.position.xPos, roverState.position.yPos + 1))
      case S => RoverState(S, Position(roverState.position.xPos, roverState.position.yPos - 1))
      case E => RoverState(E, Position(roverState.position.xPos + 1, roverState.position.yPos))
      case W => RoverState(W, Position(roverState.position.xPos - 1, roverState.position.yPos))
    }
  }

  def leftTurn(roverState: RoverState): RoverState = {
    RoverState(leftRotation.get(roverState.direction).get, roverState.position)
  }

  def rightTurn(roverState: RoverState): RoverState = {
    RoverState((rightRotation.get(roverState.direction)).get, roverState.position)
  }
}

RoverApp.scala

```
import DataTypes._
import RoverMachine._

import scala.io.StdIn.readLine

object RoverApp extends App {

println("Enter Direction")

def parseToDirection(direction: String): Direction = {
direction match {
case "N" => N
case "S" => S
case "E" => E
case "W" => W
}
}

val initialDirection: Direction = parseToDirection(readLine())
println("Enter XPosition")
val initialXPosit

Suggestions

-
For all your direction case object hierarchy, you are basically trying to duplicate an enum, which Scala provides no language-level support for. However, Scala does provide support for enumerations at the API level via the Enumeration class, which should be extended by new enums.

You could try something like this:

object Directions extends Enumeration {
  type Direction = Value
  val N, S, E, W = Value
}

and then your parseToDirection(String) method becomes unnecessary and can be replaced with a Direction.withName(String), a method for retrieving enum constants by name defined in Enumeration. You will also need to update your method signatures involving Direction to Directions.Direction, and add an import Directions._ before every piece of code in a given scope which uses a Direction, i.e., one import before declaring the rotation Maps, one in RoverMachine (this makes the get() call on the value unnecessary in the rotation methods), and one in RoverApp.

However, this might complicate the code a bit and reduce the type safety offered by exhaustive pattern matching, however.

TL;DR Your approach with a sealed trait and case objects is the preferred one in Scala, this is just an alternative. Note, you could also as well used a Java enum for this purpose too.

-
The one var in main() clouds the functional-ness of the code a bit. You could replace the loop with a recursive inner method in main, but there's always a better solution. Read to the end for a cohesive correction of your code.

-
String in Scala can be treated to inherit Seq[Char], as String has an implicit conversion to StringOps, which is a subclass of Seq[Char], so you don't need to convert the instructionList to an Array[Char] before looping over it, you can use it directly as a String.

-
Map.apply() is semantically the same as Map.get(), so use the former as it allows the syntactic sugar of making the entire call look like the following:

def leftTurn(roverState: RoverState): RoverState = {
  RoverState(leftRotation(roverState.direction), roverState.position)
}

(similarly for rightTurn(RoverState))

-
Rely on type inference: In Scala, it is unnecessary to declare the types of most values, as they can be inferred by the compiler, and this does not decrease the readability of your code much unless the declaration is extremely complicated or the inferred type is unintuitive/incorrect.

This makes your value declarations in main() look like (incorporating some of my other suggestions:

val initialDirection = Directions.withName(readLine())
val initialXPosition = readLine().toInt
val initialYPosition = readLine().toInt
val instructionList = readLine()

Also, in DataTypes, you can skip the generic type specification for the Map[Direction,Direction] and write just Map, the compiler will infer the generic types.

-
The code to move the rover should be a part of the RoverMachine, not RoverApp. Also, it should be defined as a function like this:

-
Drop the surrounding braces in one-line functions, it makes the code look more functional.

-
No exception handling - What to do if the user enters illegal characters? Scala is infamous for its confusing stack traces, it would be good to define code to handle exceptional conditions - a functional (recursive) equivalent of running the entire main() block in a while loop which breaks only on legal input. Also, consider throwing proper exceptions in exceptional cases, an example is in point 6.

-
Imports should be declared closest to their site of use.

-
There is code duplication between leftTurn() and rightTurn(). You could probably use a turn() function accepting a type of turn, be it indicated by a Boolean flag, an enum constant (I use this approach in the code below), or a case object hierarchy. Pattern match on the type of turn to choose the correct rotations Map to use.

Style

Naming:

You name your side-effect-free functions in a way which makes users think that they have side-effects. Instead of naming them with present-tense verbs, name them with past-tense verbs, like turned instead of turn, moved instead of move, etc.

Your indentation and bracket usage are spot-on unless otherwise noted.

Errors

-
In main(), you initialize roverState as

var roverState = initialRoverState(N, Position(initialXPosition, initialYPosition))

The initial direction is always N; although you accept the initial direction as an input, you never use it. That line should be

var roverState = initialRoverState(initialDirection, Position(initialXPosition, initialYPosition))

Rant

You hadn't tried much to be functional. If you had, then not even a single var, mutable data structure or loop would have turned up in your code, but all 3 did, so...

Most programmers hate global state, and for good reason. if there were any vars in your DataTypes object, it would have

StackExchange Code Review Q#147487, answer score: 3