patternMinor
What takes precedence: readability or elimination of side effects?
Viewed 0 times
whatreadabilitysideeffectseliminationprecedencetakes
Problem
I have a method:
You use said method like so.
That method has no side effects. But it's used inside a class:
Is it better to write the method such as above or to have a more succinct and readable code but with side-effecting methods? I'm guessing as this is encapsulated inside a class, this is actually "better" for maintainability but it is not as pure functional programming. It's an imperative style. However, the strongest argument against this is that it's more difficult to test as the tests are not as isolated.
def removeExpiredCookies(jar: CookieJar): CookieJar = {
val currentDate = System.currentTimeMillis / 1000L
jar.filter(_._2._2 > currentDate)
}You use said method like so.
var cookieJar = new CookieJar
cookieJar = removeExpiredCookies(cookieJar)That method has no side effects. But it's used inside a class:
class CookieManager {
type ExpiryDate = Long
type CookieJar = scala.collection.immutable.HashMap[String, (Cookie, ExpiryDate)]
var cookieJar = new CookieJar
private def removeExpiredCookies(jar: CookieJar): CookieJar = {
val currentDate = System.currentTimeMillis / 1000L
jar.filter(_._2._2 > currentDate)
}
def getCookie(domain: String): Set[Cookie] = {
cookieJar = removeExpiredCookies(cookieJar)
cookieJar.find(_._1 endsWith (domain))
}
}Is it better to write the method such as above or to have a more succinct and readable code but with side-effecting methods? I'm guessing as this is encapsulated inside a class, this is actually "better" for maintainability but it is not as pure functional programming. It's an imperative style. However, the strongest argument against this is that it's more difficult to test as the tests are not as isolated.
class CookieManager {
type ExpiryDate = Long
type CookieJar = scala.collection.immutable.HashMap[String, (Cookie, ExpiryDate)]
var cookieJar = new CookieJar
private def removeExpiredCookies {
val currentDate = System.currentTimeMillis / 1000L
cookieJar = cookieJar.filter(_._2._2 > currentDate)
}
def getCookie(domain: String): Set[Cookie] = {
removeExpiredCookies() //bracket to indicate side effect
cookieJar.find(_._1 endsWith (domain))
}
}Solution
(I'm making my comment into an answer.) You can separate everything that is static (not using the state of instances of
CookieManager) into a companion object (and import the object at the beginning of the class for convenience). This way, it is clear what functions use this directly or indirectly (by accessing class variables). This often helps me make my code clearer, and prevents stupid mistakes such as confuse class variables with function arguments etc.class CookieManager {
import CookieManager._
var cookieJar = new CookieJar
def getCookie(domain: String): Set[Cookie] = {
cookieJar = removeExpiredCookies(cookieJar)
cookieJar.find(_._1 endsWith (domain))
}
}
object CookieManager {
type ExpiryDate = Long
type CookieJar = scala.collection.immutable.HashMap[String, (Cookie, ExpiryDate)]
private def removeExpiredCookies(jar: CookieJar): CookieJar = {
val currentDate = System.currentTimeMillis / 1000L
jar.filter(_._2._2 > currentDate)
}
}Code Snippets
class CookieManager {
import CookieManager._
var cookieJar = new CookieJar
def getCookie(domain: String): Set[Cookie] = {
cookieJar = removeExpiredCookies(cookieJar)
cookieJar.find(_._1 endsWith (domain))
}
}
object CookieManager {
type ExpiryDate = Long
type CookieJar = scala.collection.immutable.HashMap[String, (Cookie, ExpiryDate)]
private def removeExpiredCookies(jar: CookieJar): CookieJar = {
val currentDate = System.currentTimeMillis / 1000L
jar.filter(_._2._2 > currentDate)
}
}Context
StackExchange Code Review Q#28297, answer score: 2
Revisions (0)
No revisions yet.