Kris Kemper

With the previous discussions about whether or not we should return null from a method, I think that there is a few important points that were missed. I wanted to weigh in one last time, before I personally declare this topic beaten to death.

In the example that was given, it was asked what to do have something like:

// What now? Oh Crap, we can’t have a drink at this point. We didn’t put enough money in the machine.

In the case that there isn’t enough money, we could have the getDrink() method could return null, return a Drink.NULL. In any case, we are defining an invariant for the use of getDrink(). We are promising that, in the event that getDrink() is called and not enough money was given, we are going to do one of those two things. Alternatively, we are saying that if and only if there is enough money added, will it return a valid drink object.

Also, with this example, the assumption is that we are going to check the post-condition of the operation in order to detect failure:

Ideally, I don’t ever want my code to veer from the happy path. I don’t want to ever have my code reach an invalid situation where it cannot return a drink. I also don’t want to expend much effort as a developer handling the scenario. I would rather write tests that asserts that it can’t happen in relation to the client object.

One such option, is to provide a different invariant scenario - One where we check a pre-condition:

or

The invariant is now that if needsMoreMoneyForADrink() returns false, we will always get a drink.

Personally, I like the precondition approach (in this particular example). It reads well, and it adheres to the principle of Command-Query separation. Also, in the post-condition example, null (or a wrapper) is being used to communicate that not enough money has been given to the vending machine, and that more should be added. Personally, I’d like to ask the vending machine if I’m supposed to give more money. Note that situation is similar to an iterator pattern.

Along the way, Andy referred to using exceptions because they follow the Principle of “Tell, Don’t Ask.” I don’t believe that it qualifies as a good representation of that principle, but I have an example a little later that I believe is better.

Andy’s example of throwing an exception when getDrink() is called under conditions when it shouldn’t have been raises an important question for me. Should we have a method that fails, and then communicates failure back to the client object (such as with throwing an exception)? Perhaps we should not have a method that is expected to return a drink at all.

In the world of object to object communication, there are only a few ways for an object to respond to the communication of another object, with each having strengths and weaknesses.

1.) You can return a value, or an object reference. Null is the equivalent of reponding with “No, I can’t or won’t give that to you.”

2.) You can return some kind of communication object or value, one that perhaps allows you to communicate a number of message types. I consider a Null object a variation of this. You could also return an enum, where each enum value represents a different type of message.

3.) You could set some kind of global variable to hold a message - something that can be read by the client object. This is generally considered terrible OO, but I see it from time to time in the form of Singletons.

4.) In languages like Java, you can throw an exception. Exceptions generally signify an exceptional error situation. Exceptions can be handled, but if not, will continue to bubble throughout the code, interrupting every called frame, finally interrupting the flow of execution if not handled.

5.) You can respond with a method call. This is the standard “Proper” way of having objects communicate. Of the different message/communication types available to object, this one is the richest and most intentional (most explicit). You can send a message (a method call) and send parameters as details with the message.

I generally avoid using exceptions to communicate between objects. I try to reserve them for really exceptional situations. I will spend time looking at important code and deciding on how to do it well, but many, many time before, I have written methods or functions that just return null, and I also wrote the code that used it. It’s simple, and people who look at that kind of logic will be familiar with it, so it’s not going to be a surprise.

Anyway, I wanted to weigh in on a different way of “handling nulls.” I created a sample solution to the VendingMachine::getDrink() dilemma; one that doesn’t assume that there is a getDrink() method at all. It’s an example of the VendingMachine responding to the client object (a Person) with a richer object communication (and a better example of “Tell, don’t ask” than using exceptions). Imagine that the interaction begins with someone calling Person::getARefreshingDrink() and passing in a VendingMachine.

I’m not arguing that this is optimal either, but I got other blog posts to write, and stuff to do. It’s meant to represent a way of having objects interact in a rich manner. I’m sure you can imagine other ways to make this better, such as moving the Person method definition into a “Drinker” interface.

I recently solved a mysterious memory leak puzzle in a java swing application that I’m working on. The source of the memory leak was proving elusive, even with people on the team using JProbe in order to find the source of the leak.

At the root of the problem, the Dialog was too complex and with too much logic to easily pinpoint the problem. Circularly referential object graphs were everywhere. I had to rip out most of the dialog in large chunks until I reduced it down to a simple JDialog, and then considered each possible factor.

I was a little tripped up along the way. Apparently, calling Runtime.getRuntime().gc() doesn’t necessarily collect memory, but if push push the JVM to the point of an OutOfMemoryError, it’ll do everything in it’s power to get all the available memory back. Pushing it the edge was a sure way to get an honest answer.

Normally, when Java memory leaks happen in Swing, the culprit is a listener of some kind. It’s easy to register a listener and forget about it, while it continues to point to other objects with a strong reference. It’s further complicated by the fact that it’s not always obvious when a reference is created, like in the case of defining an anonymous inner class.

In my case I discovered that the culprit of the memory leak was a JDateChooser object that we are using. That object is defined as part of the JCalendar api. Specifically, inside the dialog, a PropertyChangeListener was anonymously created and registered to the JDateChooser, creating a circular reference from the JDateChooser and the dialog.

So, what was JDateChooser doing? Sure enough, in the constructor, it was registering a listener with a Swing singleton called MenuSelectionManager. That singleton never dies, and does not releasing the reference so that the garbage collector can do it’s magic.

The code in the JDateChooser constructor:

I love the comment. It’s just too bad that the forum user didn’t recommend constructor injection of that manager, which would have allowed me an elegant means of preventing the memory leak without modifying the library.

For instance, I could make a WeakReferenceMenuSelectionManager that stores the listeners using WeakReferences - thus allowing the garbage collector to reclaim the objects in the event that there are no other objects with a strong reference to the listener.

Unfortunately, when the maker of JDateChooser realized they were causing memory leaks, they came up with this solution:

This is better than nothing, but is still far inferior to a WeakReference approach. This is an excellent example of why we shouldn’t write code that reaches out and calls a Singleton. statically referencing an object does not allow me to reconfigure the object without changing the source code - something I would prefer not to do in a library. Luckily, the JCalendar api is covered under the LGPL, so at least changing the library was an option.

The better fix was this. First, I create a weak change listener as a proxy around the original change listener.

Then, In JDateChooser, I changed the constructor like so:

Finally, I wanted to clean up the remaining WeakReference object. Now that the JDateChooser will get garbage collected, I can do that by creating a finalize method.

I posted this solution to the JCalendar forum, hopefully, it’ll get added to the next release. The cleanup() method is not an obvious solution. As a user of a widget api (especially in Java), I wouldn’t expect that I need to free up any resources, and indeed, a lot of time and money was wasted with developers looking into this issue.

Oh well, at least I got to have some fun with Weak References.

If you work in Java, and you don’t use a decompiler, you haven’t lived.

Well, maybe that’s a bit excessive. Still, I’m shocked by the number of developers that don’t use a decompiler like Jad combined with their IDE, like with Jadclipse and Eclipse.

In eclipse, you can download the Java api source code and attach it that so you can always look at the actual code - original comments and all.

However, you want to be able to look at the source code for any jars that your application depends on as well. I didn’t even realize how useful and freeing it is to be able to do that until I started. I would say that less than 30% of all developers that I’ve met use a decompiler, but it’s definitely one of the top plugins you have to have.

Recently, Andy Palmer post a blog article called returning null considered dishonest.

I felt that the topic started to go down the road of enforcing a contract in order to prevent nulls. Checked exception are an example of explicitly adding new behavior to the contract. The contract details a response to certain parameters in the form an exception.

Taking a step away from checked exceptions, you have unchecked exceptions. Unchecked exception that are typed are especially pointless, as they weren’t declared in the first place, and generally shouldn’t be explicitly handled. If you do want to make contract enforcement with unchecked exceptions, you can throw RuntimeExceptions or do an Assert.true() at the start of the method to make sure that a method is called with then needed preconditions.

My issue is that you’re adding behaviour - behavior that was probably added somewhere else already. Every line of code added incurs an extra bit of cost in terms of maintenance. Super defensive coding strategies should be justified due to the merits of the given situation, but not used as a default behaviour.

Null’s are okay when they are not meant to be handed out as a meaningful response. Once you try to use null values to convey a value or a meaning, then you start seeing a proliferation of nulls and null checks.

For instance, I’m not advocating this:

Nor am I advocating this:

I’m essentially advocating this:

Don’t check for null, don’t throw exceptions. Just make sure that nulls don’t occur at the entry point to the system. Most of the time, you should just write your method to do stuff and make sure that the error conditions are handled in one place as early as possible to their introduction into the system, such as when the user enters some data.

Most code bases are not published to the public. There is a big difference between published api’s and internal code bases. With a public api, you have to create more of a contract, because you want to give your providers something that they can depend on. There are too many consumers for immediate communication and collaboration. In an internal codebase, you are your own consumer and supplier.

My point here is that I disagree with adding more code complexity to fix a problem that is generally a behavioral issue. Having to handle every possible unwanted condition of a method parameter becomes a method tax, creates duplication, and results in a lot of tests that aren’t testing the interesting stuff.

I recently blogged about preventing the NNPP through better hiring practices. Though I feel that a strict hiring practice is key to ensuring consistently higher quality developers, I wanted to talk a little bit about what comes after an employee is hired.

Companies should have a serious view of employees as an asset. One that should be about individuals being nurtured and grown into something and someone greater. This is a large part of why I advocate pair programming, because there is no better way to raise the skill level of developers than have them constantly working hands-on with other skilled members of their profession.

I think though, that there is a more important point here that is difficult to define: a company has to have a culture of wanting the best people and facilitating the continued growth of those people. It’s so intangible because it doesn’t come from a CEO proclaiming a set of values that came out of a management book. It’s about what the individual people in company actually believe and embody.

Using Thoughtworks as an example again, I was initially very surprised at how ever present the culture of the company is. It’s telling that we call ourselves thoughtworkers. We identify as part of a group, not just as employees of a company. We want to be part of the group. I’ve met people that fit the culture and continue to refer to themselves as Thoughtworkers even after they’ve left the company to move on to some other career pursuit. In particular, an aspect of the culture is that of excellence.

How did this come to be? Well, I can’t say for sure - it’s an evolutionary process. However, I think that it happened for the same reason that many organizations develop a culture of excellence, whether it be great sports teams or great universities.

First, the organization has to try to select the best. There has to some amount of pride in getting in. Then, once you get in, and you meet many people that are more skilled than you (or perhaps, strongly skilled in different ways), and you see how you can improve. In fact, you want to improve - you want to live up to the standard of excellent that the organization has set. You don’t want to consider yourself below average in your group.

In teams and organizations with really strong and successful cultures, it’s not just the leadership that wants everyone on the team to be superlative, but it’s the team members as well. At Thoughtworks, I’ve met other thoughtworkers that give me suggestions on books that I should read, or technologies that I should learn. I’ve developed student/teacher relationships with some people, and I’ve developed teacher/student relationships with others. With many, it’s been peer to peer, but with us debating and sharing views and opinions.

I use Thoughtworks as an example of a strong culture, but I don’t want to make it sound like it’s perfect. It has it’s flaws too, but as an organization, it’s the best of what I’ve experienced. On the other side, I’ve worked with/at companies that have what I consider to be a poor or negative culture, but my ranting about that will have to wait for another day.

Jay Fields recently published a blog post where (amongst many other points) he mentioned the concept of the Net Negative Producing Programmer, referred to as NNPP.

I’d never heard that specific term before and I enjoyed the read. Almost every project that I have worked on had a healthy number of people who’s efforts were borderline negative to the teams productivity. I say borderline because I’ve never actually measured it - but I have worked with people that would take a week to do a days work, and it would be defect ridden when they claimed that it is done.

The part that struck me about the paper though was that it said that dismissal of an NNPP is a last resort. I do agree with the paper that it argues against measuring the person with absolute statistics in making this decision, but on collaborative/agile projects, it’s usually easy to spot the weak links. I can understand that it can be expensive and risky to fire someone, but software teams must have a way of weeding out these individuals, and removing them. I don’t just mean moving them to another team; I believe that they should be laid off. I wouldn’t even feel too bad about it, IT is still a high demand industry, and even the worst of the NNPP will find some giant company to disappear into.

There is a better way however, one that will prevent companies from getting into the position of having to worry about this in the first place. Improve the hiring standards. If you can keep the NNPP’s out of your company in the first place, you don’t have to worry about firing them.

At Thoughtworks, the interview process is pretty rigorous, there are phone interviews, a code submission, logic tests, and lengthy in-person interviews with people of varying roles. The highlight for me is the code submission. It’s pretty telling from looking at a code submission whether the candidate is worth pursuing in the first place. I don’t care how many languages and technologies someone claims to know - if they can’t create an elegant and working solution to a short problem when the expected outputs are provided, then they most likely aren’t top notch.

There is also a scale to the problems, from easy to hard. Personally, botching the easy problem is unacceptable, but I’m a slightly more forgiving with the hardest problem. In general, the reviewers are usually pretty harsh, so, it generally takes a good solution to even be considered acceptable. Oh, and experience level is factored in - junior candidates have to write pretty good code, but an experienced candidate needs to nail the code submission.

It’s a HUGE differentiator. Every company that is serious about staffing the best talent (or even just good talent) should require a code submission.

I’ve even heard of some offices doing pair-programming during the interview. This is also a great idea. Someone may know how to write great code, but still not be good at actually doing it. I’ve paired with (non-Thoughtworks) people that were supposed to have 5+ years of experience and watched them struggle to write a method with a single for-loop and get it to compile. Alternatively, I’ve paired with people that are like some freakish melding of man and machine, writing with blinding speed, and what they write is beautiful. If I owned a company or ran an IT team, I’d do my best to get the freakish cyborg artist types.

I came across some code the other day that made me wish more people were aware of the concept of Command Query Separation. It’s a great default behavior for developers to have and results in code that support more reuse.

Since I think someone out there would be upset if I posted the actual code, here is something else I made up:

Looks simple enough. I’ve got a form, and when everything is valid, the button becomes enabled and the user can move on to something else. The getValidationErrors() method gets me a list of errors if the form has any error. Oh, but wait, it does more:

The code’s contrived, and only loosely based on what I saw. What I saw was much more egregious. The point is that the method getValidationErrors() is building a menu and collecting validation errors.

What if I want to find out if everything is valid later on? Well, I’ll be rebuilding the menu again. It get worse when you see the code get more complex like so:

Sigh. I just want to know if the form is valid. I don’t want the advertisement to refresh above the form. What I really want is a getValidationErrors() and a updateMenu() and a refreshAdvertisementAccordingToRules(). Alternatively, I would have accepted a getValidationErrors() and a doABunchOfUiChangesInResponseToFormValidation().

As you may have guessed, the code that I saw was not tested either.

I have a suspicion that some developers (even some experienced developers) create code like this because they feel that they are writing the least amount of code possible (you save lines by not creating more methods!). Developer awareness aside, probably the top reason this tends to happen is lack of collective code ownership. A developer has to add a feature to a part of the system that “belongs” to someone else, so they add they try to add as few lines or if-statements as possible. They see that there is logic that they need, and they see that it is executed at the time they want it to execute their code, so they pile on their logic and finish their task.

From the example I’ve shown, it may look like a small evil, but the amount of if-else statement and total coupled logic tends to build up quick.

Conway’s law essentially states that the structure of a software system will reflect the structure of the organization that makes it.

I’ve seen this effect in small companies with little structure and who seem to be constantly “putting out fires.” This is probably the easiest one to point out, but the code base is often highly chaotic and unstructured. The opposite of that is that rigidly structured organizations have rigidly structured code.

I worked on a project once in a large, very hierarchically structured organization. Sure enough, the code base had a hierarchical structure where modules depended on other modules in a way that could almost be put into an organizational chart. Additionally, the company had divisions that interacted on a contractual basis as though they were seperate companies, and that extended to the software teams. Each team would integrate on a software “contract” basis, and it looked bad if one team “broke” that contract.

I remembered this again the other day when another developer was telling me of a manufacturing company that he worked for. He described how data was often processed from one database and stored in another, and at some point, a cron job would start a process that would pick the data up, do something with it, and store the results somewhere else for some other application. It sounded a lot like how a manufacturing plant works.

I’m sure there is a wide variety of reasons why Conway’s law happens, but I see a couple of primary reasons. One is that whatever theory of management a company implements will almost certainly extend to the software development management, as in my example of a hierarchical organization. The manufacturing example is potentially different. In the case of manufacturing software, I think that it could be the domain that results in the structure of both the management and the software. In the first case, the structure of the software was imposed upon it because it suited the management structure, and in the second case, both the management and the software are structured in order to support the business processes that need to happen.

I’ve worked on projects where we did pair programming most of the time, and I’ve been on projects where we only pair on specific issues, and as soon as it looks like we can get back to our “own” work, we do.

On a project where I’m not pairing, I miss the way information flows about the team. My favorite as a developer are the practices that other developers use as they work; The various programs that they prefer, or other tools that they discover. Most of the IDE shortcuts that I know, I learned because I paired with someone who used it, and I asked them to teach it to me. I find that there is constant learning process as a developer where someone will have some tidbit of knowledge that they share with you. Best practices are easily shared, from CSS tips to database practices.

If you do pair programming in a team of collaborative and knowledgeable people, it’s like being in university every day, but it’s all projects and no exams. Sometimes, you are the teacher, and other times you are the student. Either way, the rate of personal learning and growth is greater than if you worked alone. It’s a fun experience.

There is another important flow of information that is missed without pairing. When software grows to a certain size, the complexity of the code base (and the domain) becomes an increasingly difficult issue to manage with teams that don’t pair. Different design patterns are commonplace and entire areas of the system are built that some developers never know exist, let alone are able to immediately work on. Inevitably, a “silo” effect happens where people tend to work on what they have worked on before.

When a team successfully does pair programming, every developer will eventually be exposed to every area of the system. Every pattern will be scrutinized, and it’s strength and weaknesses better understood. The business value of the features being worked on will be understood by everyone.

People that have information that is not shared with the rest of the team become centralized stores of information. How often had we needed to talk to one person to complete our work, but couldn’t get any of their time? I’ve even seen meetings and other sessions around one or a few people trying to disseminate information to everyone at once, through handouts and power point slides. Such attempts are an ineffective way of sharing information.