Skip to content

The Amazing Adventures of a Sexy Software Engineer Posts

Git tip: back button

Do you want to go back to the previous branch you were in?

 

Leave a Comment

Looking at the wrong abstraction

Warning! The post title might be a little bit misleading. Or not. Read on, and decide.

In the coming paragraphs I am going to look at a few different issues like Swift-Objective-C interoperability, Swift extensions, and the one that actually provides the post title: how relying upon the right abstractions improves code quality and therefore maintainability.

The problem

Wait! Sexy Software Engineers do not have problems, Sexy Software Engineers have difficult solutions!

The difficult solution

I hate (not) to make this about me, but I spend most of my time dealing with native clients that consume content from online video platforms (aka OVPs). In other words, I spent an awful lot to time building lists of Movies, TV Shows and whatnots.

For this particular difficult solution, I need to display a list like this:

Screen Shot 2015-02-23 at 13.10.47

The list contains two different kinds of “stuff”: movies and tv shows. To make things a little bit more (opening air quotes) interesting (closing air quotes), both Movies and TVShows are objects modelled as good old Objective-C classes, and I am not allowed to modify the source code of any of those two classes.

The difficult solution solved: the ugly way

This is the simplest possible solution. In the implementation of the listing datasource, I can do something like:

Ugly, yes, but Marines don’t do, Marines make do, and so on and so forth.

Now, seriously, this is more evidence supporting a concept I am trying to get across in almost every post. Start with something simple, something that gets the job done. Then look at it carefully, with a critical eye, and you will see all the code smells and bad design decisions jump off the screen, right towards you.

What’s wrong with that? Why is that ugly?

Here is the culprit: isKindOfClass. If you write Java for a living, it would be instanceof. In Swift, it would be the is operator.

YMMV, but I am going to say it as clear as I can: checking for a type at runtime is a code smell. A nasty, stinky code smell.

I know, in theory, there is a reason why isKindOfClass, instanceof and similar are part of the frameworks. In practice, I still have to find myself in a situation where relying on checking the actual type of an object does not stink, and I still have never found myself in a situation where checking the type at runtime was the best possible solution.

How can this be solved in an elegant way?

Well, as usual, taking a step back, and looking at the big picture. What is it that I am trying to achieve? Displaying Movies and TV Shows in the same listing, right?

Yes, and no. What I actually want to do is list a collection of strings. I should not care about what is actually producing those strings, about what it actually is, I should only care about how it behaves.

That is the key concept here: your abstractions would not focus on what things are, but on how things behave.

So, back to my example. If I shift from Movies and TVShows, to something-that-holds-the-data-needed-to-be-presented-in-the-listing, my listing would only have to use one entity, one abstraction.

Step 1. Declare a protocol.

I assume naming might be off. The goal is providing a method that returns the content that we want to display in the cells, already formatted properly as a single string.

Step 2. Declare a couple of extensions to Movie and TVShow

Extensions can be used to enforce protocol compliance. This is what The Book has to say about that:

Extensions add new functionality to an existing class, structure, or enumeration type. This includes the ability to extend types for which you do not have access to the original source code (known as retroactive modeling).

That is just what we did. Without touching Movie and TVShow, we enforced them to implement a protocol. Even better, notice how Movie and TVShow are written in Objective-C, while the extension is Swift. Neat!

Step 3. Implement the listing data source in Swift.

This is necessary. Only Swift code can use the new features in Swift (like Generics or Extensions). So this one is kind of a no brainer. But Swift is fun, so I call that a win-win!

Step 4. Consume the protocol type, instead of the previous object types

That means, that in the listing’s datasource you can just do:

No casting, no if branches. The listing deals with just one type, that type being the ListItemPresenter protocol.

Step 5. Review the solution

Is this a better solution? Indeed, it is. Checks for instance types are flaky and easy to break. Also, what would happen if we needed to display another entity in the same listing?

We have certainly increased the amount of code, but by doing so we have made the solution more robust and easy to maintain.

Summary

Sometimes the trees do not let you see the forest. Sometimes seeing the forest hides the fact that we are looking in the wrong direction, staring at abstractions that make things more complex for us.

Usually, what your entities expect are other entities that behave in a particular way, without actually minding about what those other entities actually are.

When building your abstractions, look for behaviours.

4 Comments

Principle of least surprise

Another Principle! I am on a roll! Although, to be honest, this is not an actual principle.

If you work for a serious company, you would most likely have one of these printed out and hanging from one of your office’s walls. If you don’t have it, now you have something to do today.

wtfm

When it comes to source code, this principle can be summarised as:

Don’t try to be a smartass. Write code that is easy to understand.

Oh, noes! Is this another rant about semantic code?

Actually, no, it is not, thanks for asking. I would not say it is a rant, I would say it is a more like unwanted advice, but about something else.

The offending code.

This code, slightly edited to protect the innocent, was extracted from an actual codebase my team has to maintain.

There are plenty of WTFs here. But the one that stands out the most is on smartMethod: why on earth would I loop all the entries in a Map only to act upon those that match a set of constant, immutable, known keys?

Isn’t our life complicated enough, as it is? Why do we want to make our code more difficult to read and understand, for the shake of a flexibility that may or might not be necessary? A flexibility that, at least in this example, is not even flexibility at all?

Do the simple thing first.

Instead of looping the map keys looking for something we already know is there… why not just ask for it?

 

Now, if you look at that code carefully, if you look at any piece of simple, dumb code, carefully, you will instantly hear it screaming at you, shouting everything is wrong with itself.

In this example, the smartMethod has an expectation on which keys in a Map it will need to use. That tells you that a Map is not the right type for that parameter: maybe two strings would do?

Summary

Keep it simple. Do not try to impress anyone. Seriously, your skills as developer are not directly proportional to how obscure your code is.

Leave a Comment

Dependency Injection will make you look cool

I love this quote:

“Dependency Injection” is a 25-dollar term for a 5-cent concept.

I love it so much that I posted it separately.

What on earth is Dependency Injection?

Every time I have to explain what is Dependency Injection, I can’t avoid picturing myself as one of those cheap comedy characters, torn between two little instances of myself siting on my shoulders, one wearing all white, the other one wearing all red.

The one in white is whispering to my ear “provide some background, start the Dependency Inversion Principle”. The one in red,though, will be jumping up and down, shouting “tell him about passing everything your class might need in the constructor”.

Yeah, I have a rich complex inner life.

The Dependency Inversion Principle

Let’s do the right thing, and start from scratch.

A. High-level modules should not depend on low-level modules. Both should depend on abstractions.

B. Abstractions should not depend on details. Details should depend on abstractions.

There you have it, the Dependency Inversion Principle in all its glory. Obviously, if you throw that to anyone’s face, you should expect at least some eye-rolling, if not some head banging (them banging your head against the nearest wall).

In a nutshell, what the principle states is that both high and low level components should depend on the same abstraction. More about that later…

Riiiiiight, but what is Dependency Injection?

Basically, it is a pattern. Not one of the patterns in the GoF, but a pattern nonetheless. A pattern employed to facilitate that both high level and low level components depend on the same abstraction, aka the Dependency Inversion Principle.

Riiiiiiight, would you mind providing an example, smartass?

First, no problem. Second, mind you colourful language, please.

But you have a point. Here is an example of a very simple multi-layer app. There is a service layer that performs operations against an specific service (a service might be an actual remote service, or just a file on disk, let’s assume for the shake or the argument that, in this case, it attacks a remote service), and a business layer on top of it that provides a domain-specific API.

Quite straightforward, right? Indeed. The problem here is, what happens if we need to change the way the service layer works? What if we want to grab the configuration from a file on disk, instead of a remote service, or even worse, decide what to do at runtime?

Yeah, we are in a bad situation. Why? Because there is a hard dependency here. The ConfigurationManager depends directly on the ConfigurationService. Those two classes are tightly coupled, the manager can not work without the service.

Sweet. You are good at describing code smells. What about a solution, please?

Sure, no problem. Dependency Injection to the rescue! No, wait! Dependency Inversion Principle to the rescue!

First, I will try to find an abstraction both the manager and the service can depend upon.

Second, I will make my service implement the ConfigurationService protocol.

Now here comes the part where we all facepalm in sync, because suddenly, the concept “Dependency Injection” makes sense. The Dependency (in this example, remember, it was the ConfigurationService) will be injected into the ConfigurationManager as a parameter in the constructor. But wait, there is more! The ConfigurationManager will not hold a reference to an specific dependency, but to an abstraction (the Configuration interface).

This last bit is important, so I am going to repeat it. The ConfigurationManager depends on an abstraction now. It is not aware of which actual service it is using, and it actually does not give a damn about it.

Summary

Dependency Injection decreases coupling, and improves the flexibility and modularity of all designs. And it is a neat term that will make you look cool if you use in front of your colleagues.

As usual, you can grab the source code on GitHub

1 Comment

The Open-Closed Principle

Today we are going to go through one of the five tools that every Object Oriented Designer, or just anyone not wanting his code to become a loaded gun pointing at her head, should have in her toolset: the Open/Closed Principle.

The Open/Closed Principle is one of the most mentioned, but yet most misunderstood, of the five SOLID principles. I am sure you are fed up with reading the definition all around the interwebs, but here it is anyway:

Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.

Say what!?

Open for extension, but closed for modification. Neat. But, what does that mean? Let’s rephrase the Principle a little bit:

We should be able to modify the behaviour of a piece of code without changing the actual code that implements it.

In other words, we should have magic powers, or superpowers, or be able to bend the space-time continuum.

Having superpowers would be neat. Can I have some?

Sure, it would be neat. But it is definitely out of the scope of this post.

Let me rephrase. How can my code be open for extension but closed for modification?

Let’s go through a real life example. Real life, like in “code extracted from an actual codebase my team has to maintain”. Let’s pretend that the fact that I personally wrote the offending piece of code does not matter.

The problem

I mostly work with OVPs (that’s Online Video Platforms). I build those pesky apps that some TV providers deliver to their subscribers, and allow said subscribers to watch TV on their devices.

So, I needed to display a Program Guide. An Electronic Program Guide, actually.

An Electronic Program Guide is a list of what is going to be broadcasted during a particular range of dates/times. Let’s pretend this is what I need to display:

Screen Shot 2015-04-03 at 8.29.42 pm

Bear with me on this one, please.

As you can see, the list needs to display Movies, TV Shows, and Live events. When the user taps a particular Program, the app needs to transition to a “details” view, which is, obviously, different (different content and different layout, and even different business logic) according to the type of Program the user selected.

For historical reasons, and some practical reasons, the model object that was provided with in order to build that listing looks similar to this:

Yes, you can argue than all my troubles will end if I looked at the right abstraction, and if I did not need to rely on that “type” property. And I would agree with you: remember, there are historical reasons behind this. Anyway, all this “look at the right abstraction” thing is probably matter of another post.

But, on the other hand, relaying on the right abstraction does not change the fact that I need to navigate to different views according to the type of content I need to display.

The simplest possible solution.

I am a big fan of keeping things simple. When you keep things simple, code smells, bad designs, and bad solutions just popup in front of you when you look at the code, jumping out of the monitor, waving their arms, screaming “refactor me, refactor me!”

So, the simplest implementation that would get the job done would be:

Riiiiiiiight, a switch.

Switch is bad

No matter what they tell you, no matter what you see, switch clauses are usually a code smell. And in this particular case, ermahgawd, it is just bad. I wrote that code, and I still want to punch myself in the face overtime I see it. But why?

Well, this will not escalate well. First, and most obvious, Program is poorly designed, it is the wrong abstraction, and to workaround that bad design, it exposes a type to help identify what it actually represents. That is clearly meant to change.

But, the worst is about to come. What if I need to deal with a new type of program? I would need to update that switch. There is no way to extend this code that does not imply editing the navigateToDetails function.

This code is not open for extension and closed for modification.

Again, the simplest possible solution clearly exposes two code smells: first, Program is the wrong abstraction, or at least it is poorly designed, and second, the initial design is not easy to extend.

The better solution, open for extension and closed for modification

Let’s assume that Program is not going to change now. It will, but not at this precise moment.

If you look at the first implementation of the navigateToDetails function, you will see how all the business logic that deals with navigation is in there. That function checks the program type, and launches the command needed to navigate to the desired destination. What I want, is find a way to delay those decisions as much as possible. That way, I can deal with the problem in a generic way here.

The first step to fix my problem (remember, I want to be able to navigate to different views according to the type of Program the user selects) starts with declaring a protocol that abstracts the details of how I want to perform that navigation.

Then, I will have different implementations of that protocol. Each of those concrete implementations will deal with the details of how to navigate to the desired destination, and more importantly, they will provide a way to decide if that particular implementation is supposed to handle a Program or not.

That last part is the key of the whole refactoring. Notice how I can inject (or in less fancy words, pass as a parameter) a list of all the available implementations of the Router. My navigateToDetails function will loop those, passing them the Program. If it finds any implementation that can deal with the Program passed as parameter, it will ask the implementation found to proceed and navigate to… where? That’s the thing! The navigateToDetails function does not know where, because it doesn’t need to know.

 

Delay all decisions!

I recall reading, some time ago, that good Object Oriented Design consisted in finding ways to delay decisions. I only agree with that up to a point (I guess I have been a victim of over-engineered systems more than once), but I think it makes a good rule of thumb. If a particular part of your system knows too much about others, maybe it is time to rethink it.

But I digress. The whole point of this post was implementing a solution to a problem in a way that allows easy maintenance, and simplifies scalability.

Summary.

Object Oriented Design is all about trade-offs. Simple code usually gets the job done, but sometimes simplicity in the code brings along poor maintainability. Sometimes, ten lines of code get the work done, but sometimes you need to substitute those ten lines by a hundred lines, as long as those hundred lines provide a solid, clean and maintainable solution.

And, did I mention that unit testing the second solution is way easier?

Code samples.

You can get a couple of Swift playgrounds containing the source code to this post on Github

Leave a Comment

Code reviews are awesome, when done properly

Code reviews are awesome. Seriously, if you are not doing code reviews you should start doing them right away. There are many good reasons for doing so.

Code reviews are a learning opportunity

When done properly, code reviews are awesome learning opportunities for junior developers. There is no better way of growing as a developer, than having someone well seasoned in front of you, explaining to you, line by line, how to make your code better.

But wait! Code reviews are awesome learning opportunities for senior developers as well. You know that saying “an old dog does not learn new tricks”, don’t you? Well, there is nothing better for a senior developer than a newcomer to the craft asking all kinds of questions about a codebase. If makes you reconsider each and everyone of your decisions, and most times, it makes you realise how many things you do that you should probably not be doing. And how you do those things without any particular reason.

Code reviews help improve the code quality

This should be obvious, but apparently it is not. The main goal of a code review is being sure that the code quality is up to the team’s standards. Now, what are those standards might be a theme for another post (or a thousand posts), but the key concept here is that opening up the code for review, everybody gets to read it, discuss it, and improve it through suggestions and comments. Which leads to…

Code reviews enforce collective ownership of the code

You have probably heard it a thousand times: code should be owned by the whole team. There is no better way to make the team feel like the code is owned by everyone than making everyone actually own the code. And that happens when everyone’s suggestions and comments are incorporated into the code base. More importantly, opening up the code for review avoids what I call dark zones, chunks of code that none has actually seen, and that no one knows what they actually do or how they actually do it. Again, collective ownership FTW!

Code reviews help share knowledge about the code base

Here comes Captain Obvious! Yes, if everybody is supposed to read everybody else’s code, understand it, and being able to comment about it and suggests improvements to it, as a side effect, that means that everybody understand the whole code base. And that is awesome!

Code reviews avoid nasty surprises.

We have all been there. James did work on the new feature, and his code was complete. Until we tried to integrate Jame’s feature into the new release. That’s when we found that, apparently, James has a different definition of “code complete”. Code reviews will rise to the surface issues with the code as early as possible. If the feature is not implemented as it is supposed to, a good code review will catch that.

Code reviews motivate developers to write their best code

If you know that everyone else is going to read your code, you are going to find that extra pinch of motivation to avoid duplication, or to come up with better names for your methods and variables, or to break down that very long method into smaller methods.

Code reviews create a team’s culture

Yeah, the company culture. That thing every company on the face of earth wants to create, that thing that is supposed to act as a mesh, as a framework, that supports all the engineers, that binds them together, that makes easier for new hires to catch up and blend into their teams. A healthy system of code reviews makes wonders when it comes to create that. There is no better mentoring for a new developer that his first pull request open for review. There is no better way to make that new hire feel as he is growing as a developer than receiving continuous feedback about his code and being able to provide feedback to his colleagues about their code.

All that is cool, and everything, but how can you make sure that everybody understand code reviews as all of the above?

Well, I am very sceptic about this whole Scrum and Agile Cult. But there is something in that process that I really like: the idea that teams should be self-organised. Give a team the opportunity to figure out how and why they want to do their code reviews, and they will come up with something good. Good, I say, because it will be something they all will stand by, and thy will all abide by.

That is what my team did, And this is what we came out with (edited to remove some company-specific items)

Code reviews: rules and regulations

Rule #1

Code reviews are a learning opportunity, both for the person opening up his code for review, and the persons doing the actual review, and they should be approached as such.

Rule #2

Code reviews are done for one reason: deliver the best possible code.

Rule #3

Comments in the code reviews shall be performed with rules #1 and #2 in mind.
Corollary 1: Comments in the code reviews should be backed up by arguments and facts, not by personal preferences.
Corollary 2: Refrain from adding comments that provide no value. If a single comment can achieve the same result of 10 comments, it is better to make that one comment vs. 10 different comments in 10 different places.

Rule #4

The result of a good review should provide:
– A list of things to update. There should be a clear definition of what to update.
– A list of things that require further discussion.

Rule #5

In order of importance (descending) a review should focus on:
– Global intent of the code (e.g. does it do what it is supposed to be doing?)
– Architecture (e.g., does it follow the technical design?)
– Gaps in the unit tests (e.g., does it cover all the requirements?)
– Code readability (e.g., does it conform to code clean guidelines?)

Corollary: While code style is also an integral part of code readability, it is not considered an important aspect of the code review unless it blatantly violates the agreed upon standard.

Leave a Comment

Quote

Confidence is a valuable, but fragile, resource. If it’s not fragile, it’s no longer valuable.

Kent Beck

Leave a Comment