# Scala Pitfall: Parameterless Function Calls and Misplaced vals


I've been using Scala for the better part of a year, and it's mostly been an
enjoyable experience. Scala fits in a comfortable position in the programming
latent space somewhere in between Java, Python, JavaScript, and Rust. However,
Scala is definitely a a "big" language -- it has lots of language features,
supports many programming paradigms, and has a large enough surface area that
the likelihood of encountering one (of many) footguns is _high_.

One pitfall that I've seen fairly regularly is in the (mis)use of parameterless
methods. In Scala, likely due to its functional programming influence, functions
without parameters do not need parenthesis. So, the following call patterns
function identically and are both valid:

```scala
class Dog {
	def bark() = { println("bark") }
}

val dog = new Dog()
dog.bark() 	// With parens
dog.bark	// Without parens
```

By itself, this isn't a harmful feature. Invoking method functions without
parenthesis, in my opinion, introduces a little bit of cognitive load -- since
you don't always know if what's being evaluated is a function, or just a member
variable.

What can get you in trouble though is that if one swaps out `def` for `val` in
the above example, the behavior changes significantly, with just one keyword
change.

Take the following example, creating a very basic counter object:

```scala
class Counter {
	private var count = 0

	def increment() = {
		count += 1
	}

	val value: Int = {
		count
	}
}

c = new Counter()
c.increment
c.increment
println(c.value) // Prints 0!
```

In this case, it's pretty clear what's going wrong -- the `val` block of `value`
is only evaluated once, when the `count` is still 0. However, from the call
site, this isn't clear, since `c.value` could be a member `var` or `val`, or
could be a parameterless function call (`def`). In languages that _don't_ allow
function calls without parenthesis, the distinction would be more clear:
`value()` indicates a function call, `value` indicates a member access.

In a less trivial example, this is more-or-less something that I've seen happen
multiple times in my current code base. For this example, assume we're pulling
some configuration data from a cache -- for instance, whether or not to enable a
feature or not. This could be to implement a dynamic kill switch for a
particular set of features.

```scala
class CacheClient {
    def getValue(key: String) = { ... }
}

class MyConfiguration {
	val cache = new CacheClient()

	def isFooEnabled: Boolean = {
		cache.getValue("foo")
	}

	// Oops!
	val isBarEnabled: Boolean = {
		cache.getValue("bar")
	}
}

class BusinessLogic {
	val conf = new MyConfiguration()
    def handleRequest() {
		if (conf.isFooEnabled) {
			// Some code path
      	} else {
			// Another code path
        }

		if (conf.isBarEnabled) {
			// Some code path
		} else {
			// Another code path
      	}
    }
}
```

The issue above, with `isBarEnabled`, is that it _looks_ dynamic, in the same
way that `isFooEnabled` actually _is_ dynamic, but it's not! If the underlying
value in the cache changes, `isFooEnabled` will correctly (dynamically) fetch
the value from the cache each time, but `isBarEnabled` won't. It just saves the
value of `cache.getValue("bar")` that was returned at initialization, and never
re-queries the cache.

From the `BusinessLogic` code though, you'd never know this, unless you dug in
to the implementation of `MyConfiguration`.

My team owns something that looks pretty similar to the `MyConfiguration`
object, as part of a common library that gets used in most of the services
running at my company. Since these methods are, by definition, only used on
configuration objects, they aren't typically unit tested. Additionally,
integration tests often fail to catch this sort of issue, since they don't
usually test the value of the configuration changing at runtime.

To solve this particular issue, my team added a wrapper around the dynamic
component, and provide a `getCurrentValue` method to get the value dynamically.
This makes it a bit harder to misuse:

```scala
class ConfigurationFetcher[T](key: String, default: T) {
	def getCurrentValue(): T = { ... }
}

class MyConfiguration {
	val fooEnabled: ConfigurationFetcher[Boolean] = ConfigurationFetcher("foo", false)
}

class BusinessLogic {
	val conf = new MyConfiguration()

	def handleRequest() {
		if(conf.fooEnabled.getCurrentValue()) {
			// Some code path
       	}
		...
	}
}
```

We also added static compile-time checks to prevent a common misuse:

```scala
class MyConfiguration {
	// Disallowed: ConfigurationFetcher is not a class property!
	val fooEnabled = {
		val fetcher = ConfigurationFetcher[Boolean]("foo", false)
		fetcher.getCurrentValue()
	}
}
```

Of course, there are still ways of getting around the static checks, such as
wrapping the output of `getCurrentValue` _itself_ in a non-changing `val`:

```scala
class MyConfiguration {
	val fooEnabled: ConfigurationFetcher[Boolean] = ConfigurationFetcher("foo", false)
	val barEnabled: ConfigurationFetcher[Boolean] = ConfigurationFetcher("bar", false)

	// Not caught by static analysis, but won't work as expected!
	val fooOrBarEnabled = fooEnabled.getCurrentValue() || barEnabled.getCurrentValue()
}
```

This isn't really a _problem_ with Scala, rather just one outcome of its
flexible nature and terse syntax. Your eye eventually becomes trained to see
`def` and `val` as _importantly different symbols_, but even so some of these
issues are tricky to spot.