8
\$\begingroup\$

I've recently begun learning Scala, and while I've run into its concepts before (immutability, tuples, first-class functions) I'm not sure whether I'm using the language how it's supposed to be used. My learning project is going to be a simple little dungeon crawler, and while you can see the code on GitHub, I'm copying a simplified version here below.

Example output looks like this, where ?s are potential rooms and #s are actual rooms:

     ?     
 ??  #  ?  
?## ### #  
  ### ####?
  ?? ## #  
      ##   
      #    
    ?##?   
     ??    

And here's how it's implemented:

class LevelBuilder(random: Random) {

  /** Mapping of positions to the rooms at them. */
  var rooms: mutable.Map[(Int, Int), Room] = mutable.Map()

  /** List of areas where rooms could be generated. */
  var possibilities: Seq[(Int, Int)] = List()

  /**
   * Adds a new room to the map at the given position, and adds the available
   * positions around it to the possibilities list.
   */
  def addRoom(position: (Int, Int), room: Room): LevelBuilder = {
    rooms += position -> room

    // First, remove the room from the list of possibilities.
    possibilities = possibilities.filter(pos => pos != position).++(around(position))

    // Next, add the positions around that one to the list...
    possibilities = possibilities ++ around(position)

    // ...but then remove all of the ones around *those*, so no two paths will ever link up.
    possibilities = possibilities.filter(pos => roomPossibility(around(pos).count(p => rooms.contains(p))) && !rooms.contains(pos))

    this
  }

  /**
   * Determine whether a room should be build based on its number of neighbours.
   */
  private def roomPossibility(neighboursCount: Int): Boolean = {
    neighboursCount <= 1
  }

  /** Adds the given number of rooms to the level. */
  def addRooms(number: Int): LevelBuilder = {
    for (i <- 0 to number) {
      var index = random.nextInt(possibilities.length)        // Pick a random room from the possibilites list...
      addRoom(possibilities(index), Room.randomRoom(random))  // ...and add it
    }

    this
  }

  /** Return the four positions around the given one. */
  def around(position: (Int, Int)): Seq[(Int, Int)] = {
    List(
      (position._1 + 1, position._2),
      (position._1 - 1, position._2),
      (position._1, position._2 - 1),
      (position._1, position._2 + 1)
    )
  }

  /** Return a build layout of this level. */
  def build(): mutable.Map[(Int, Int), Room] = rooms.clone()

  /**
   * Print out a grid view of the level so far, showing both possibilities and
   * created rooms.
   */
  def debug() {
    val miny: Int = rooms.minBy(_._1._2)._1._2 min possibilities.minBy(_._2)._2  // there's got to be a better way to write this
    val maxy: Int = rooms.maxBy(_._1._2)._1._2 max possibilities.maxBy(_._2)._2
    val minx: Int = rooms.minBy(_._1._1)._1._1 min possibilities.minBy(_._1)._1
    val maxx: Int = rooms.maxBy(_._1._1)._1._1 max possibilities.maxBy(_._1)._1

    for (j <- miny to maxy) {
      for (i <- minx to maxx) {
        if (possibilities.contains((i, j))) {
          print("?")
        }
        else if (rooms.contains((i, j))) {
          print("#")
        }
        else {
          print(" ")
        }
      }
      println("")
    }
  }
}

So what do you all think? I've only been using Scala for a few days, so I'm interested to learn things like whether I should prefer using operators to methods, or whether there's a certain style of variable names I should be using, or anything like that.

\$\endgroup\$
4
\$\begingroup\$

Looking quickly over your code here are a couple of the changes I would make:

First, I would add a class to your application (probably within the levelbuilder.scala file) that encapsulates the (Int, Int) tuple you are using throughout this code snippet. E.g.

    case class Pos(x: Int, y: Int) {
      def neighborhood: List[Pos] = {
        Pos(x + 1, y) :: Pos(x - 1, y) :: Pos(x, y - 1) :: Pos(x, y + 1) :: Nil
      }
    }

When building this class I also took the chance to rewrite the method called around(pos: (Int, Int)) from the LevelBuilder class to a more concise method which I've called neighborhood just to differentiate it.

Also, if you haven't had a chance to read-up on case classes in Scala, now would be a good time to do so :) They provide a lot of functionality with just a few lines of code.

Now you may declare the LevelBuilder fields rooms and possibilities like so:

    val rooms = scala.collection.mutable.HashMap[Pos, Room]()
    var possibilities = List[Pos]()

One place that these changes drastically add to readability in your code is in the debug() method:

    def debug(): Unit = {
      val possibilePos = rooms.keys ++ possibilities

      val minY = possiblePos.minBy(_.y).y
      val maxY = possiblePos.maxBy(_.y).y
      val minX = possiblePos.maxBy(_.x).x
      val maxX = possiblePos.maxBy(_.x).x

      // print map 
    }

Notice also that I changed miny to minY etc.

\$\endgroup\$

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.