Scala: The Million Trait March…

Traits are arguably one of the nicest things about Scala.  Doubly so to anyone who has had to deal with the strict OO-rigidity imposed by Java.  Component driven, loosely coupled systems are much better than rigid inheritance based systems.  Interface abstract out implementations.  On the other hand, sometimes there’s functionality or method patterns that need to be replicated over several classes which make sense existing as a mixin.

It’s one of those powerful things that when used by the right hand simplifies code but in the wrong hands… well, we’re back to multiple inheritance.  So here are my observations of classic pitfalls of trait abuse or misuse, followed by why.  It’s my own code smell repository.

Your Class is Too Large For One File

One of the biggest complaints I’ve heard with the Active Record pattern is that it handles too many concerns.  It not only represents itself as an object but also reads and populates itself from a database (every single time it’s created to boot!)  Conceptually it’s supposed to represent in code the current state of the database at the time of creation as it relates to the object’s structure.  So maybe it’s not doing too much after all.  This is something I’m not trying to debate here.  I bring it up because it leads to a very good example of too much code in one class.

Now imagine that you had a class designed to work with several data stores, such as Reddit, Mongo, MySQL, and a simple file, but you wanted to capture the magic of Active Record.  Moreover, you weren’t just looking to capture the magic of Active Record with a single data store a one time but all of them at the same time.  All this code (and there would be a lot of it) would look like a bad code smell.  Using traits, you could break up the functionality into several files and thus cut down on the appearance of this smell.  Your inspiration?  Microsoft!

You see, the creators of C# did something great, or stupid depending on how your company/colleages allow it to be used.  They designed the language and compiler to allow a developer to split a single class definition into multiple files.  My guess is that is was just too easy to put in, much like null was in C.  You know how well that turned out.  Traits can accomplish the same thing, breaking a class into several files.  Resist the temptation.

I said you were cutting down on the appearance of a bad smell not cutting down on the bad smell itself.  There’s a big difference.  If your class can’t fit into a max of three screens (there are always exceptions,) then it’s probably doing too much.  A class with 21 pages of code makes just as much sense as a class with 21 pages of code split up into several files spread all over the place.  Refactor and reduce.  Specialize if need be.  Pimp your own library.  I don’t care.  Just don’t make a mockery of all your hard work (or a puzzle for the next guy to figure out.)  Here’s a few red flags you might hear if you’ve done this or are about to be handed code like this:

1. “We separated the class into several traits, otherwise it would just be too large…”
2. “You can find the details of that class in these files…”
3. “If IDE keeps running out of memory on that file, split it up into several traits…”
4. “Just search for any file that contains a self type to that class…”

The Convenience Methods Have Taken Over

Sometimes it’s nice to be able to call not only “toString” but also “toJson” on any object.  Go ahead and mix it in.  And maybe you want to also be able to selectively call “toHTML,” “toXML,” and “toSomeCommaDelimitedFormatJoeMadeUp.”  Now your class has two concerns: what it was designed to handle and how to express itself in several different string formats.  That’s one too many and you know it.

Mixing in helper traits like this seems easy and feels like it’s reducing effort.  Technically the only thing you’ve added to your code is a “with Blahblahblah.”  The class’s core function and purpose isn’t obscured by a forest of method implementations at least not in the source code (right?)  With proper documentation, you could add whatever trait you wanted just to make your life easier as a developer.  That’s the power of traits and part of why they were created.  As an aside you’re only following the same conventions as the language designers themselves, right?

A rigid adherence to OO-design principals sometimes stands in the way of good, extensible and flexible code (think “with Logging” as a prime example.)  I’m not arguing against classes with thick interfaces as long as those interfaces promote the utilitarian nature of a class.  Scala has thick interfaces in many core classes (by “thick” I’m referring to the number of methods.)  Take a look at the standard List.  That’s a lot of methods.  I stopped counting at 20.  The important point is that the implementation details lie elsewhere and they’ve added almost no other dependencies to the class.  If you can achieve the same, great.  It takes a lot of effort to pare it down to the level they pared it down to while at the same time yielding so much functionality.

But therein lies the dark side.  Most people I know don’t have the time or development budget to spend doing this.  Instead, they introduce tons of dependencies and/or snippets of code which clutter and obscure the purpose of a class.  Other times the “core” of a class is actually implemented in one of the traits with a few core variables and/or method implementations placed in the source file.  The actual class consumes so few lines of code that even a minimal amount of helper functions obfuscates what that class is supposed to be doing.

It’s a fine line to walk.  Just like above where the lines of the class no longer fit into a single screen if the lines of “helper” methods outnumber the number the lines of core concept methods in the source file a future maintainer is going to have a harder time understanding what your class is trying to do.  The “core” should be plainly obvious.  Any helper method should be transparently obvious.  If they aren’t it’s time to start thinking about refactoring your code or pimping your library.

Here’s some telltale signs:

1. “Most of those methods don’t have anything to do with using that class.”
2. “The only important method you need to worry about is…”
3. “We added that to make it easier to use with that library and those with that library…”
4. “I forget where it’s supposed to do that.  Just start looking in some of the traits…”

The One Method Used Mixin

There’s a method on a trait that’s handy.  I mean, really, really handy.  It’s so handy that you’ve mixed it in even in places where it might not make much sense.  Sometimes it might have made sense in the past but after a couple years of growth you only need a single method of that trait.  That trait, however, is used as a argument type in several other functions that have nothing to do with where it’s being used.  It’s essentially multiple-inheritance.

You know the reason they disavowed multiple inheritance in Java?  Some programmers were making coffee pot/plane hybrids because they wanted temperature controls.  Never mind that the plane could also make a cup of joe.  That was just something they would never use.  But yet, a plane now required you to add fuel and coffee just to get off the ground.  The sales pitch “and it doubles as a coffee pot” would go about as far as you think it would in a used plane auction.

You need a single method from a trait.  Perhaps you need a subset only.  Don’t debate the merits of wholesale trait removal, refactor it out.  The great thing about traits is that they can extend other traits.  Strick to what you need and nothing more or KISS (Keep It Simple, Stupid.)  Tell tale signs of the one-method-used-mixin:

1. “I just needed that method from that Trait.  I’m not really using any of the others…”
2. “You wouldn’t actually try to use it in that function, I just put it in there because it makes life simpler…”
3. “No, it’s not really related to those classes, I just wanted to use it in this one place for one thing…”

Concept Enforcement

A trait without any code, methods or functions which discriminate based on that trait is nothing more than an empty shell.  I’m not talking about empty sealed traits (which can and do play pivotal roles in compiler checked pattern matching.)  I’m talking about the type of trait someone cooks up to represent an abstract idea but puts in no machinery to support it.  That is, there’s no compiler plug-in or static code analysis tool backing up that concept.

They’ve added the Immutable and Mutable traits in to the Scala library.  If there was no compiler plug-in to enforce it then someone could instantiate a “new StringBuilder with Immutable.”  Good luck with that.  The same can be said of your trait.  That “NoThrow” trait?  I’ll inherit it and add in a method that only throws exceptions.  Maybe it’ll throw a randomly selected exception to be spiteful.

It’s a noble goal you’ve set for the code; a trait that signifies something special.  Maybe it’s something you want the junior or future developers to respect so you put it in code (comments rarely stay current, right?)  Stop and think deeply if you hear yourself saying:

1. “I wanted to enforce that it had to have this property…”
2. “It’s more of a lable…”
3. “It doesn’t do anything.  Just don’t make any class extending it do…”

If you can’t take time to build a compiler plug-in or add in a post compiler code analysis tool to enforce your conceptual trait, write documentation.

The Dependency Web

You see this?  That’s what happens when every class depends on nearly every other class.  You know how simple it is for this to happen in Scala?  Sadly, the answer is with very little effort.  The problem is that without knowing which classes extend a trait, some traits can develop “extra” methods.  These extras are needed by a subset or even a majority of all mixed classes but not by all.  Moreover, they have hard-coded dependencies on other classes.  Those classes could mix in traits that depend on still more.  You see where this is going, don’t you?

It’s very hard to have loose coupling in a system where you don’t always have all the information in front of you.  Even with it, method signatures are sometimes enough to couple systems in surprising ways.  In Java, we’d try to reduce that by making abstractions.  Sometimes we’d have to resort to the adapter pattern (wrapper pattern) or refactor out common pieces of interfaces.  That was good for Java (Scala too) and I’m not advocating against it (not much.)  I definately don’t want my Scala to look like my Java and I’ve discovered it doesn’t have to.  Functions being first class citizens and structural typing (think duck typing on steroids) remove not only a ton of boilerplate but also the need to rely on some of the GoF patterns (when used right.)

Structural typing reduces coupling.  How?  Traits, unlike plain interfaces, let you declare variables without assigning them.  While this does provide one more avenue for strong coupling to creep into your code it also lets you declare variables which are functions that work on structurally typed entities rather than interfaces, other traits, or even hard dependencies to another library.  If what you need is an object just so you can call a method on it, make a method of type “Unit => A.”  If you need several methods, introduce a type.

I separated this out from the “Convenience Methods” section above because I’ve seen first hand (with my own code) what careless coupling can do to a project.  However, it is often these convenience methods which cause the trouble in the first place.  It’s great to add a “with UseStatistics” method to your classes but you can inadvertently introduce a hard dependency on some library (like Twitter’s Ostrich) without meaning to do so.  Maybe you’re still using Configgy but now you’ve got both.  Anyone taking your “helper” trait now has to include it plus all of its dependencies.  Maybe your trait, as helpful as it is, makes unit testing difficult!

Signs of strong coupling are simple:

1. Your dependency tree looks like a ball of yarn (see link above.)
2. You can’t remove the dependency on a library even though your group has decided to deprecate it’s use.
3. You can’t easily unit test a class because the list of mocked classes you have to create is longer than the Great Wall of China.
4. You need a configuration file to test a very simple class.

The One Class Used Trait

As libraries grow, designs chance, requirements change, delivery dates change and people leave/join the development group bloat creeps into code.  It’s often unavoidable given development budgets (time budgets) or priorities of a business.  In a perfect world this wouldn’t happen but when it does, it’s good to see it recognized.  That said, sometimes good intentions go awry.  I’m talking about traits that weren’t designed to ever be used except by a single class (not class hierarchy.)

If you conceived of a trait that’s only going to be used with a single class either in instantiation some of the time “new Foo with Bar” or always “class Foo extends Bar” then get rid of the trait and put it in the class.  Traits were designed as a way of compartmentalizing functionality that was to be shared among several classes.  They were not meant to be self-typed to a single class which is never intended for any hierarchical embellishment or family.  If you let this go eventually you may wind up with a class consisting of 27 different traits, spread over 27 different files, with 27 screens of code.  Now you’ve really got a code smell.

Think about pimping a library.  Think about pulling the core functionality of a case class into a trait and having two versions of that case class.  Think about your design and why you want a trait that is only ever used by a single class.  Think about your design and try to avoid saying:

1. “Yeah, it’s only used by that class sometimes but I wanted to avoid another implicit…”
2. “I kept it as a trait because it doesn’t make sense to be included with the code…”
3. “I wanted to make the class source clear so I moved out those methods into a trait…”

What you’re really saying is that you either didn’t understand the design you were about to make or that your class should be split up.

Conclusion

I’m sure I’m missing several other abuses of traits.  In fact, I bet I’ve not gone in depth enough on some of the points above to make my intent clear or concise.  There’s a lot of leeway in writing Scala since the community as a whole has not converged stylistically to one way or another.  Some groups might practice “good” code development which other groups find abhorrent.  It’s quite clear that there’s a lot of people who are moving towards a strictly functional approach while others are moving more towards a mixin, immutable OO approach.  I don’t know where I am at the moment.  I like both.

That said, I do know that there are some useages of traits which drive me up a wall.  Good developers know when they can break the rules and when the rules have been broken so much a refactor is in store.  I’d like to be a good developer.  This is my “refactor time” check list.  My reminder to myself that I need to make more use of structural typing, that I need to stop throwing a million helper traits in to make a feature “just work this one time,” and that I need to formalize my thoughts on what isn’t good code.  It’s taken me a lot of time to think and see how others use traits to come up with my own opinions.  Unfortunately, these are just my opinions and are largely self-directed.

If this helps or you have different opinions, share them.   I’m not afraid to say I was wrong and I truly believe I can learn from junior developers just as easily as I can teach them a thing or two (maybe just one thing.)