Wednesday, October 7, 2009

Code Consequences

I think we're all aware that our actions have consequences for ourselves and, often, for others. I'm going to talk about such a situation that is relevant to iPhone developers.

I've had to turn down some development work while writing More iPhone 3 Development. This isn't a big deal, as I expected that to happen when I agreed to do the book, and I like to see it, because it tells me that our platform is still doing well. However, a surprising number of the projects that I've turned down have been to fix iPhone applications written by other contract developers, something I don't consider to be such a good sign.

Even if I were available to take the work, there's something inherently uncomfortable about these situations. Some developer whose identity is unknown to me wrote this code. That developer has no chance to respond to my criticisms and I, in turn, have absolutely no idea of the situation under which the code was written. I feel like there's no right way to point out flaws in code like this. Yet, to fix the problem or even give an accurate estimate, those problems have to be identified.

In one such recent instance, the potential (and very trusting) client actually sent me their code along with the description of the problem. I knew that I couldn't take on a hefty project until the book is finished, but the description of the problem sounded like something that I might be able to be fix quickly and easily, so I looked at the code that evening.

The problem was exactly what I thought it was going to be based on the description, and I thought I'd be able to fix it with a few dozen lines of code over not more than a couple of hours, which would be a win for the prospective client and the client's users who were experiencing significant performance problems with relatively low volumes of data. In a well-designed application, fixing this specific problem wouldn't have impacted anything outside of a single class or, at worst, a handful of classes. It was a bottleneck in pure "Model" code in the MVC sense. In theory, as long as I didn't change the way the object's data was accessed and updated by other classes, I could change the implementation details, such as how the data was persisted, without impacting anything.

You know what they say about theories, right? They work great, in theory. But in practice…

So, yeah, that theory didn't pan out. I started looking at the rest of the project looking for potential cross-depenencies and I found that my assumption was totally and completely wrong. The underlying data store wasn't only vended through the data class, it was accessed directly by literally dozens of classes. In fact, all the actual persistence code - both loading and saving - (with the exception of encodeWithCoder: and decodeWithCoder:) was contained in controller classes. And there were dozens of these controller classes, many of which were nearly identical except for a handful of lines, which seemed to indicate a general lack of design or forethought. The core problem was thus made significantly harder to fix by a desperate and unrequited need for refactoring. There were, literally, several dozen classes that could have been written as a single-class or, at worst, a couple of subclasses with a common parent. The entire project looked like one thrown together by a tragically inexperienced developer, or one who didn't have any use for "this new fangled OO shit". Seriously, there should be a link to this project on the Wikipedia's spaghetti code entry, in the section on "Spaghetti with meatballs". Reading the project made me feel like I was in some weird programming equivalent of Poe's Law; I don't think I could intentionally create code this convoluted.

Now, I wish I could say that I'd never seen nor written bad code, but that wouldn't be true. I've seen lots of bad code over the years and have written more than I'd care to admit. Despite that, though, this code was worse than most of what I've seen, and that's saying something, given that my job for several years was fixing problems in other people's code.

After a couple of hours, I came to the inescapable conclusion that this job was beyond my ability to take on right now. I'm almost positive that it would be more work to try and fix the multitude of problems in this code than it would be to just rewrite the application from scratch. Even if not, the end result would certainly be better. Out of fairness to the prospective client, I had to explain why I had to turn down the project, something that seems likely to cause problems for the previous developer. I couldn't, in good conscious, not tell him, however. I did put it in far more diplomatic terms than I am doing here in my blog, at least, where I sometimes personify the fact that 'tact' is a four letter word.

The high demand for iPhone developers has made it a profitable livelihood and, I think, has caused many people to offer their services as iPhone developers without truly adequate experience. Hell, I've been working with the SDK as long as anybody outside of Apple, and I sometimes feel like I don't have "truly adequate experience" with it - that's the peril of a new technology I guess. Regardless, when you're developing for a client or employer rather than yourself, you really need to be aware that there will be downstream developers affected by the decision you make or fail to realize are there in the first place. When you cut a corner to save time or fail to use good design because you don't know any better or don't care, that decision may very well have significant consequences for your client and for other developers who inherit your code.

Good application architecture and writing good code takes a little longer in the short run, but in the long run, it requires much less of a time investment to maintain. Yes, I know most of us bill by the hour and writing bad code is far more profitable than writing good code, but don't do that. Seriously. That's worse than just being ignorant of how to write good code. Despite the economy, there's no shortage of iPhone development work right now, and even if that weren't the case, you owe it to your clients to give them the best code you are capable of writing.

Addendum: Based on some of the comments, I fear that this post has come across as rather more judgmental than intended. Contract software development, especially for clients who are not familiar with the process, is extraordinarily hard. No contract developer has ever, ever created a perfectly designed, bug-free application. In the real world, you have to deal with deadlines, unreasonable demands, and tons of other factors that work against you delivering a perfect application. I am not unsympathetic to this at all, as I have experienced it myself on countless occasions.

I intended this only as a cautionary tale to give you something to think about when deciding whether to cut a corner or to skip doing that rewrite you know you need to do, not as an indictment of anybody. Believe me, looking at some of my early contract work, I've got some pretty harsh words for myself.

One of the most important traits in a developer is the willingness to accept that no matter how good you are, you do make mistakes. You will never stop making mistakes and you will never stop being able to learn from those mistakes. Sometimes, you can even learn from others' mistakes and save yourself some pain.



10 comments:

schwa said...

Blame the developer why doncha. ;-)

Because customers never want apps to be written fast and cheaply (oh and bug free too!)

Jeff LaMarche said...

I seem to recall saying I find being asked to fix other people's code uncomfortable for precisely that reason, and I'm well aware of the difficulties of writing software for clients who don't understand the process by which software is developed. I've lost money on a few projects by failing to realize just how clueless the client was about the process. I've cut corners to meet project schedules, or implemented a less-than-perfect architecture because the client failed to mention some important fact, and I failed to elicit the information early enough.

That all being said, this particular case is beyond anything that can be attributed to schedule or unreasonable client demands. I can't imagine any situation in which it would be reasonable for someone holding themselves out as a professional developer to produce code of this caliber. I've seen your code; believe me, we're not talking the same ballpark. We're not talking the same city. At its worst, your code is good. This code isn't even just run-of-the-mill bad.

This is a developer who delivered an application that needs to store and display thousands of objects, yet falls apart with a few dozen. And the problem can't be easily fixed because the problem isn't an isolated bug or bottleneck, it's a problem with the entire design. There's really no way to avoid a do-over.

No matter the conditions, a developer who produces code that can't be used and can't be fixed has to take some of the responsibility for producing it. We can't expect clients to be reasonable, but that doesn't relieve us of our obligation to create code that works and can be maintained.

Gargs said...

Could you elaborate more (or provide a link for further reading) on the downfalls of using the controller classes for persisting data? I just got started with this, and going by Apple sample code, it seems that the preferred way is to have the controller classes call the save method (on the managedcontext or the SQLlite DB). Even the examples in your book seem to have all the persisting code embedded in the controller classes.

Would really appreciate some guidance on this. I am sure it would help all the other budding developers, too.

Thanks!

brandon said...

I completely agree with pretty much every point you've made.

We all create technical debt, there's no way around it. But it is how we react to cleaning it up and the circumstances in which it was created that separates good developers from mediocre developers.

There is no excuse for delivering a prototype as production though. It sounds like that might have happened.

Jeff LaMarche said...

Gargs:

It's a good point.

One of the difficulties with writing a book on programming, especially one targeted at beginners, is that you constantly have to balance good design with simplicity. Simple code examples are easier to understand; good design often obfuscates the thing you are trying to teach. Sample code has to strike the same balance given that it's intended for teaching. If it's too complex, it's hard to understand. If it oversimplifies, it teaches bad habits.

In Beginning iPhone Development, we usuallly erred on the side of simplicity. In More iPhone Development, we're erring more on the side of good design because it's intended for a slightly more advanced audience who already has some of the basic building blocks.

If you're using Core Data, it's not really as much of issue because the code to do the save is contained in the Core Data classes. Your controller class merely calls save: and you're done. The actual persistence code - the code to insert into the database or save to the filesystem - is contained in NSManagedObject and the other Core Data classes.

When you're designing your own applications, you should follow a similar model. Objects should know how to persist themselves, and class methods on that object should allow you to find and load objects from disk. The controller class shouldn't have code to actually open a file or execute SQL, but it will, of course, have code that triggers suc code.

Unfortunately, I don't know of a really good resource ons high-level application design. More iPhone Development will focus on good design much more than Beginning did, but we primarily use Core Data for persistence there.

Hope that helps. At some point, I'd like to do an article or series on high-level design, but it won't be anytime soon, I'm afraid.

Jeff

Jeff LaMarche said...

Brandon:

Yeah, I didn't want to come across as too judgmental here, though I fear I might have based on schwa's comment above.

Absolutely, this stuff is hard. Being a contract developer is not an easy job by any stretch of the imagination, and you never, ever create a perfectly designed, bug-free application. Ever.

But, there is a standard we need to hold ourselves too, and that means, at time, pushing back on the client and saying "no" to unreasonable requests. It means educating the client to some extent.

It's not easy and it's not always fun, but that's part of the reason it pays well. If it were easy, you wouldn't see rates in excess of $100 an hour. If you're going to take that kind of money, though, you need to be worth it. It sometimes means putting in extra unbilled hours to get something right. It means constantly educating yourself. It often means working miracles.

brandon said...

Its not always the fault of the developer. I have been in a few squeezes with clients that ask for one thing and when you laying out the design of the app, you try to ask all the questions that could come up in the future, but when they see there app all those answers to those predicted questions get reversed. This leads to either a major rewrite of code or it not in there budget to spend on these rewrites, and when its not in the budget they ask for you to cut corners... and then maybe a year down the road some other developer looks at it and thinks bad of the original guy (me). I'm sure someone has seen my code and thought "what was this guy thinking", but its what the client wanted.

jsd said...

I know what Jeff is saying. I have volunteered to clean up a few projects and if you were to see the code your brain would just explode. I'm not a genius OO programmer by any stretch of the imagination but even I can recognize some of these rookie mistakes.

The iPhone's open development/app store model is both a blessing and a curse. The curse is you have a ton of people with dollar signs in their eyes ready to dump any old piece of crap in to make a quick buck. These people are willing to hire any inexperienced newbie to throw something together quick and on the cheap. As long as it works well enough to part a few suckers from their $0.99 then who cares how well designed the code is.

Luckily I am being paid a real salary by a real company to do real iPhone work 40 hours a week, but I realize not everybody is in the same boat.

nskboy said...

It would be nice if you will share your tips and tricks on how we should not design/develop applications. For example, your first post will be based on review of the application you're talking about here.

Glenn said...

This is a great post. I myself have been working with the SDK, or pretty much Cocoa for less than a year but already I've noticed other developers purposely cutting corners just to make the so-called "quick buck". I don't know how well their app does when published but like you mentioned, in the long run I doubt the gains overtake the costs. Like everything in life, balance is preferable.