# Refactor to reduce code duplication

I have four methods like these (here are only two of them):

  def checkLeft(clickedIndex: Int): Option[Int] = {
val leftIndex = clickedIndex - 1
if (leftIndex >= 0 && clickedIndex % field.width != 0 && isEmptyCell(leftIndex))
Some(leftIndex)
else
None
}

def checkRight(clickedIndex: Int): Option[Int] = {
val rightIndex = clickedIndex + 1
if (rightIndex < field.size && clickedIndex + 1 % field.width != 0 && isEmptyCell(rightIndex))
Some(rightIndex)
else
None
}


They all have similar structures. How I can reduce code duplication here?

• Does bottomIndex depend on clickedIndex? And what something1 depends on? – Petr Pudlák Jul 22 '13 at 16:44
• @PetrPudlák I edited question with real code. Please review it – MyTitle Jul 22 '13 at 16:49
• Pretty sure in the second block you'll want parens around clickedIndex + 1 before the % operator. – Carl Manaster Jul 22 '13 at 17:02

For the LEFT case, you're checking that an index is above the low value of zero; for the RIGHT case, you're checking that it's below the high value of field.size. You can't really reconcile these except by combining them into a method that checks both boundaries - and there's nothing wrong with that. Now you've got

if (inBounds(theIndex) && isEmptyCell(theIndex) && ...)
Some(theIndex)


That ... is questionable to me because you seem to be comparing a different index (clickedIndex or clickedIndex + 1) depending on LEFT or RIGHT.

I think there are errors here that you need to reconcile before going too far down the path of duplication elimination.

One way to do this would be with a trait or abstract class defining the basic behaviour of a check. Then you extend that for the actual checks you want.

trait indexCheck {
def adjustedIndex( index: Int): Int = index
def newIndexCondition(index: Int): Boolean = true
def oldIndexCondition(index: Int): Boolean = true
def apply( clickedIndex: Int): Option[Int] = {
if (newIndexCondition(returnIndex) && oldIndexCondition(clickedIndex) && isEmptyCell(returnIndex))
Some(returnIndex)
else None
}
}


Then you create checkLeft and checkRight classes, overriding methods as necessary

class checkLeft extends indexCheck {
override def adjustedIndex(index: Int): Int = index - 1
override def newIndexCondition(index: Int): Boolean = {
index >= 0
}
override def oldIndexCondition(index: Int): Boolean = {
index % field.width != 0
}
}

aCheck = new checkLeft
aCheck(clickedIndex) // The magic apply method is used here.


My example above is probably a little over-specific. It would be more flexible just to have a conditionList onto which could be pushed a sequence of functions returning booleans. But you get the idea.

By the way, I used traits in case you wanted to be able to create classes which did radically different things but also implemented checks. However, if you made indexCheck a class rather than a trait, you could do things like building maps of anonymous classes, like this:

val checks:Map[String,indexCheck] = Map(
"left" -> new indexCheck {
override def adjustedIndex(index: Int): Int = index - 1
override def newIndexCondition(index: Int): Boolean = {
index >= 0
}
override def oldIndexCondition(index: Int): Boolean = {
index % field.width != 0
}
},
"right" -> new indexCheck {
override def adjustedIndex(index: Int): Int = index + 1
override def newIndexCondition(index: Int): Boolean = {
index < field.size
}
override def oldIndexCondition(index: Int): Boolean = {
Index + 1 % field.width != 0
}
}
)


Which gives you a map of anonymous classes, which you can use like this:

checks("left")(SomeValue)
checks("right")(SomeOtherValue)


or whatever.

Another way to do this would be through higher order functions. That is, methods can return functions just like any other value/object. So you could have a method that took various parameters (some of them would have to be closures) and returned a function that would implement a specific check - a check factory, if you like. I'd need to know more about the parent object to give a useful demo of that.