Collected Wisdom

This should have been started a LONG time ago… It should have many more pearls. But as they say,

The best time to plant a tree is 20 years ago. The next best time is today.

Type aliases vs. opaque types

You often have some kind of data that has a particular role but a common type, usually String. For example, a Userid. Using a more specific type such as Userid makes code easier to read/understand/maintain.

You can define type Userid = String, but the compiler doesn’t actually enforce it.

An alternative is an opaque type. See oat.xplatform.models.forms.FormDecRole as an example. The advantages are:

  • It helps make the code easier to read/understand/modify.
  • It’s a distinct type; the compiler will tell you if you used String instead of FormDecRole.
  • Zero runtime overhead.
  • You can add extension methods (eg startsWith).
  • You can write some givens to seamlessly create instances from strings. This is particularly helpful in building test cases; try to avoid it in the production code. It undermines the purpose of having a separate type, after all!

Downsides:

  • A bit of code to write.
  • The Play Json library serializes Maps with an opaque type differently than if it was a string. FormDecRole has this issue. See FormSerializers for a solution (given roleMapFormat).

The Userid class (and others) was written in the Scala2 days. It should be rewritten using Scala3’s cleaner syntax and probably lower overhead.

Use Model-View-Controller

Never fall into the trap of “oh, it’s such a little UI; I won’t bother”. It’s worth the little bit of overhead to follow a standard design pattern that clearly separates the concerns of maintaining the data from displaying it. See the page on MVC.

Named functions are good

OAT developers have often fallen into the trap of doing do much on a data structure rather than writing a named function. For example, I refactored code today that had the following expression:

if (isFormsAdmin ||
decisionsFE.formalDeciders.exists(fd => fd.role == role && 
   fd.getApprovers.contains(this.approver)) ||
sd.lookups.get(role).exists { _.contains(this.approver)} ) { ... } else { ... }

So much better to give it a meaningful name: def isApprover(role:Role):Boolean. Even that could probably benefit from some helper functions or at least named subexpressions.

To be clear, this advice holds even for much simpler expressions than the above!