Decided to learn Scala. Here's an attempt at a Cloth simulation

I have never done anything functional before. These two functions are hideous to look at. II think the first step to do is extract the inner bits out and then map over my arrays. What else can I do to make this code a bit more functional?

https://github.com/dbousamra/scalacloth/blob/master/src/cloth/Cloth.scala

Specifically:

def verletIntegration() = {   

    for (row <- grid) {
      for (p <- row) {
        if (p.stuck) p.setCurrentPos(p.getPreviousPos)

        var multiplyByTime = Tuple2(p.getForces * timestep * timestep, gravity * timestep * timestep)
        var minusPrevPos = Tuple2(p.getCurrentPos.getX - p.getPreviousPos.getX, p.getCurrentPos.getY - p.getPreviousPos.getY)
        var together = Tuple2(multiplyByTime._1 + minusPrevPos._1 , multiplyByTime._2  + minusPrevPos._2)
        p.setPreviousPos(p.getCurrentPos)
        p.setCurrentPos(new Position(p.getCurrentPos.getX + together._1, p.getCurrentPos.getY + together._2))
      }
    }
  }

  def satisfyConstraints() = {
    for (row <- grid) {
      for (p <- row) {
        if (p.stuck) p.setCurrentPos(p.getPreviousPos)
        else {
          var neighbors = p.getNeighbors
          for (constraint <- neighbors) {
            val c2 = grid(constraint.getX)(constraint.getY).getCurrentPos
            val c1 = p.getCurrentPos
            val delta = Tuple2(c2.getX - c1.getX, c2.getY - c1.getY)
            val deltaLength = math.sqrt(math.pow((c2.getX - c1.getX), 2) + math.pow((c2.getY - c1.getY),2))
            val difference = (deltaLength - 1.0f) / deltaLength
            val dtemp = Tuple2(delta._1 * 0.5f * difference, delta._2 * 0.5f * difference)
            p.setCurrentPos(new Position(c1.getX + dtemp._1.floatValue, c1.getY + dtemp._2.floatValue))
            grid(constraint.getX)(constraint.getY).setCurrentPos(new Position(c2.getX - dtemp._1.floatValue, c2.getY - dtemp._2.floatValue))
//          }

          }
        }
      }
    }
  }

for (row <- grid) {
  for (p <- row) {

//should be written as:

for(row <- grid; p <- row) {

Tuple2("bla", 42)

//should be written as

("bla", 42)

I don't really understand why you use tuples when you have already a class for doing exactly such kind of math, namely Position.

The code could be more functional by using immutable case classes for Positions (but it depends on the problem if this is the right thing to do):

case class Position(x: Float, y: Float) {
    def -(that:Position) = Position(this.x - that.x, this.y - that.y)
    def +(that:Position) = Position(this.x + that.x, this.y + that.y)
}

Then you can write things like pos1 + pos2. Note that you don't need the getter methods, as you can write pos1.x, and no setter methods, as you can write and pos1.copy(x=12). The new isn't required any longer: val p = Position(1, 3).

How to proceed with the refactoring would depend on your decision if you keep Position as it is, or if you want to go with immutable Positions.

[Edit]

Here is how I would refactor it:

Run.scala:

package cloth

object Run {

  def main(args: Array[String]): Unit = {

    import javax.swing.JFrame

    val test = new Screen
    val frame = new JFrame("scalacloth")
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE)
    frame.setPreferredSize(new java.awt.Dimension(640, 720))
    frame.getContentPane().add(test)
    test.init
    frame.pack
    frame.setVisible(true)
  }

}

import processing.core._

class Screen extends PApplet {

  val cloth = new Cloth(
    rows = 20, 
    columns = 20, 
    gravity = 0.001f,
    timestep = 0.8f,
    fixedParticles = List(Coordinate(19,0))
  )

  override def setup() {
    cloth.createGrid()
    size(640, 720)
    background(255)
    smooth()
    noStroke()
    fill(0, 102)
  }

  override def draw() {
    background(255)
    fill(255)
    stroke(0)

    def drawLines(particle: Particle):Unit = particle.neighbors.foreach { n =>
      line(particle.currentPos.x * 20 + 20, particle.currentPos.y * 20 + 20,
           cloth.grid(n.x)(n.y).currentPos.x * 20 + 20, cloth.grid(n.x)(n.y).currentPos.y* 20 + 20)
    }

    for(column <- cloth.grid; particle <- column) drawLines(particle)
    cloth.verletIntegration
    cloth.satisfyConstraints
  }
}

Position.scala:

package cloth

case class Position(x: Float, y: Float) {
    def -(that:Position) = Position(this.x - that.x, this.y - that.y)
    def +(that:Position) = Position(this.x + that.x, this.y + that.y)
    def *(scalar:Float) = Position(this.x * scalar, this.y * scalar)
    def length = math.sqrt(x*x + y*y).toFloat
}

Particle.scala:

package cloth

class Particle(
    var currentPos: Position, 
    var previousPos: Position, 
    val gridIndex: Coordinate, 
    val restLength: Float,
    val neighbors: Array[Coordinate],
    var stuck: Boolean 
    ) {

  val forces = 0.0f
}

... and Cloth.scala:

package cloth

import scala.collection

case class Coordinate(x: Int, y: Int)

class Cloth(
  val rows: Int, 
  val columns: Int, 
  var gravity: Float,
  val timestep: Float,
  val fixedParticles: List[Coordinate]
) {

  val grid = new Array[Array[Particle]](rows, columns)

  def createGrid(): Unit = for (x <- 0 until rows; y <- 0 until columns) {
    val coord = Coordinate(x, y)
    val pos = Position(x, y)
    grid(x)(y) = new Particle(pos, pos, coord, 1.0f, findNeighbors(coord),
                              fixedParticles.contains(coord))
  }

  def findNeighbors(coord: Coordinate): Array[Coordinate] = Array(
    coord.copy(x = coord.x - 1),
    coord.copy(y = coord.y - 1),
    coord.copy(x = coord.x + 1),
    coord.copy(y = coord.y + 1)
  ).filter(isOccupied(_))

  def isOccupied(coord: Coordinate): Boolean = 
    (0 <= coord.x && coord.x < rows && 
     0 <= coord.y && coord.y < columns)

  def verletIntegration(): Unit = for (row <- grid; p <- row) 
    p.currentPos = 
      if (p.stuck) p.previousPos else {
        val multiplyByTime = Position(p.forces * timestep * timestep, gravity * timestep * timestep)
        val together = multiplyByTime + p.currentPos - p.previousPos
        p.previousPos = p.currentPos
        p.currentPos + together
    }

  def satisfyConstraints() {

    def calculateConstraint(constraint: Coordinate, p: Particle) {
      val c2 = grid(constraint.x)(constraint.y).currentPos
      val c1 = p.currentPos
      val delta = c2 - c1
      val difference = 0.5f * (1.0f - 1.0f / delta.length)
      val dtemp = delta * difference
      p.currentPos = c1 + dtemp
      grid(constraint.x)(constraint.y).currentPos = c2 - dtemp
    }

    for(row <- grid; p <- row) 
      if (p.stuck) p.currentPos = p.previousPos
      else p.neighbors.foreach(calculateConstraint(_, p))
  }

}

Hope that helps...

StackExchange Code Review Q#3553, answer score: 8