Hey y'all. Come visit me at dkeithrobinson.com
October 06, 2005 |
34 Comments
…or, You Down With OPC?
(Other People’s Code.)
Over the years I’ve had the opportunity, as I’m sure many of you have, to work with all kinds of code. Some of it was nice and clean and easy to follow. Some a bit convoluted, but well commented. And some was a total mess.
It’s one of the things that spurred me toward Web standards. While working for Children’s Hospital I inherited some of the craziest code I’ve ever seen. The Javascript was miles long, un-commented and read worse than anything Dreamweaver spits out. The HTML was beyond messy. It’s a good thing there was no CSS, because I’m sure that would have been a nightmare as well.
All of this added hours, days and possibly weeks of work to my already busy schedule. When I started there I had to work with it. Not fun. It was a blessing when I was given the go ahead to start over from scratch.
One of my goals was to leave the code in such a place that whomever got it after me would be able to get around and understand things easily. I think I did a pretty good job.
Here are a few things I know help me when I take over someone’s code. Feel free to add you’re own suggestions in the comments.
If you code your site, as best you can, to valid standards it’s going to be easier for the person who comes along after you to understand. Even if that person doesn’t “get” standards. Using valid XHTML with CSS, for example, helped me shave hundreds of lines of my code.
The less markup you use, the easier it is for me to deal with.
Using CSS to separate out the presentation layer is usually a big plus, even if it’s not perfect. I prefer pretty verbose CSS, but I’m ok with shorthand as well. I like it to be commented, but I can get around if it’s not. As well, I LOVE it when people point out their hacks and what they’re for. I usually try to avoid hacks, so there are times when I’m not 100% what’s going on.
Another good thing is to use selectors that have meaning. For example, calling the style for the right column of a page “right” or something.
Update: Sorry, my bad. That isn’t a very good example above. I wasn’t thinking. It’s almost as bad as assigning a color name, like “blue”, to your selectors. Something with real meaning, like “maincol” or “navigation” is better.
This one goes out to all the folks on the AJAX bandwagon. Javascript comes in many “dialects”. Meaning, I’ve never seen it coded the same way twice. One of my biggest challenges with OPC is that there are times when I just can’t seem to work my way through complicated Javascript.
Usually some simple comments explaining what a function does, or even where to look, help quite a bit. As with CSS, giving meaningful names to your functions helps me quite a bit too.
If you’re doing something that’s really complicated, inherently hard to grasp or fragile—please use extensive commenting either explaining how to futz with it, or a simple note why I shouldn’t touch it at all.
I think the best tip is to think about what someone else might need when you’re coding. If you know someone’s going to have problems understanding how your CSS is working for a particular element, put some comments in.
This can even help you when you’re coding for yourself. I know I’m often scratching my head about why I did something a particular way. Simply reminding yourself, now and then, to keep it simple, and comment the fancy stuff, can make a big difference.
Filed under: Web Development
Keyword Tags: markup code web+development
At my current contract job, I work with other people’s code all day. It’s amazing to me how convoluted the code can get on a such a simple looking site. At my job, I am repurposing other people’s site branding into a “skin” that can be used on our web applicatoin. It’s amazing to me how difficult it can get to understand a hive of nested tables and superfluous markup.
Just got a new client the other day whose site was completely standards based and clean. It was such a breath of fresh air!
Posted on October 6, 2005 10:06 AM | #
i’m with you on pretty much all of these. my view on comments is probably slightly different. i’m very much anti over commenting. in my experience, if a piece of code is so complicated that it needs comments explaining what it does, it should be rewritten and refactored until it’s obvious what it does. comments too often are a band-aid where stitches and antibiotics are really needed.
Posted on October 6, 2005 10:36 AM | #
OPC is no fun. One reason we’re (trying to) move to Rails - OPC will be much better, as there is a lot less of it, it is seperated logically, the 1,000 different ways to do the basics you can ignore.
Posted on October 6, 2005 10:37 AM | #
As long as I’ve been coding, one of my major goals has been to create code that any developer should be able to understand, or at least work with. I always make the assumption when building a site that someone else will be maintaining it at some point. It’s very nice to read about someone else who has a similar train of thought.
I’ll have to disagree with you on one point, however. You mention that CSS style names should have some meaning, such as “right” for a right column. While I agree in theory, this isn’t a good example. “Right” may describe that particular column’s location at present, but since CSS allows virtually unlimited changes in layout, what is currently the “right” column may be changed at any point in time. Terms related to presentation should not be used in your XHTML. For instance, if a particular DIV always contains news releases, call is newsreleases, even if it currently resides on the right side of the page. Or if the content will change, call is secondarycolumn, or whatever. But presentation should not be a part of the naming strategy.
Posted on October 6, 2005 10:56 AM | #
I used to use directional / position-related names like “right” and “left”, but have moved away from that practice, specifically because these elements can and do get moved around (sometimes in the markup, sometimes in the CSS).
I still try and use functionally relevant names (like “primary-nav”, “secondary-nav”, etc.). It’s still possible to identify the elements readily, but you won’t end up with a right-floated “left-column” :)
As far as comments - they are important, no question. That being said, I usually restrict my comments to a) areas of significant complexity or b) kludged solutions. (Ideally, all the kludged solutions would go away when the code goes into production, but let’s be honest…more often than not the budget and/or the schedule don’t always allow for that.)
Posted on October 6, 2005 11:02 AM | #
Good post. I’m dealing with legacy code now. Some of the files I’m dealing with though are over-commented - snarky remarks and overly verbose…
One thing that I noticed in your post that I don’t agree with:
“…Another good thing is to use selectors that have meaning. For example, calling the style for the right column of a page “right” or something. I’m often surprised when this isn’t done…”
This isn’t forward compatible - say that “right” column suddenly needs to be on the “left”? Wouldn’t a better name describe it’s content? Like “news” or “blogroll” or something? This is a problem I struggle with all the time - picking class/id names that make flexible, semantic, forward-compatible sense.
Posted on October 6, 2005 11:03 AM | #
re; Right and Left – Of course you’re right. No pun intended. It’s a bad example. My bad.
Posted on October 6, 2005 11:08 AM | #
Greg: Good point, but unfortunately not all HTML code is semantic. It’s fundamentally tied to how you want to display it. I agree in using more general names such as ‘sidebar’ or whatever, but even that is vulnerable to a restyling. It’s ultimately a judgement call that can only come with experience.
anders: Yes, with unlimited time we could refactor everything until it was clear, but the reality of writing code is that you don’t fully understand the problem until you’ve dealt with all the details. Usually there are dozens of changes to the specs during the development process. In the end the code can get a little ugly, but the deadline is here and no time to refactor until the next iteration. Beyond that there are many different styles of coding, so its unrealistic to expect something complex to be clear to everyone. A comment is a 5 second way of letting someone know what’s going on. Often times this results in more flexible code than refactoring something until it’s obvious what it does because it only does one thing and there’s no way to re-use it.
A microcosm of this is regexp design. You don’t write a parser where a regexp will do just because it would be clearer. Instead you throw on the x flag and add some whitespace and comments.
That said, overcommenting could be annoying, however I’ve never seen it.
Posted on October 6, 2005 11:21 AM | #
One persons code is another’s spaghetti! What I’d be interested in is how folks comment up changes. For example changes in CSS files - do you have a last modifed date and comments to what’s been changed, just comment out the old code so you can see the changes or something else?
Posted on October 6, 2005 11:23 AM | #
I am actually perfectly ok with the occasional “blue” class if you have many miscellaneous elements around the site which may be classed differently but need to appear blue for some reason. In that instance, I’d class the element with something like this:
<span class=”pullquote blue”>”I love doughnuts.”</div>
Better to use this stuff sparingly, but sometimes it really does help keep things compact.
Posted on October 6, 2005 11:48 AM | #
I totally disagree with you. Thre is a comedic value in naming all your styles things like: boner, boner4u, david-banner, r2d2 and bologna-feet.
Even better yet is the idea that when I leave my job some poor sap will have to figure out exactly the tag “bulbous-mudflaps” does.
Posted on October 6, 2005 11:48 AM | #
Mike – The problem with “blue” is if, for some reason, you need to say, change “blue” to “green”. I’ve personally seen this quite a few times.
But I think what you’re saying makes sense in your example.
Jim – Funny! ;0)
Posted on October 6, 2005 11:52 AM | #
1. This article was worth it just for the “OPC” line. Classic.
2. One class=blue: I agree that this is not the best idea. As Keith said, it gets weird if you go and change the class=”blue” items to color: green in CSS. But, on the other hand – is that really a problem? No, it works just fine – it’s just a little goofy. Bottom line: I’d avoid it, but I don’t think it’s quite as big a deal as some people make it out to be.
Posted on October 6, 2005 12:33 PM | #
Although I agree with the comments about using ‘right’ and ‘left’, c’mon, how many real world examples are there were a column suddenly needs to get switched from right to left?
I’ve not come across such an occurrence, and therefore don’t see why it’s so bad to use these types of descriptions. Why plan for something that never going to happen?
Posted on October 6, 2005 12:58 PM | #
Hmm, I guess you could say the same thing goes for PHP. I can’t stand when people’s PHP code is so horrendous and un-commented it’s pretty much impossible to decipher. Now, I’m no PHP wiz, and I’m sure if would be easier if I was, but c’mon. If you’re going to distribute a script, you should at least have a little commenting/documentation going on.
Posted on October 6, 2005 01:10 PM | #
These are good rules for when you arent coding for other people as well.
I’ve worked on the same site since 2000, and when I open a page I worked on back then, as a beginner, I don’t know what the heck I was doing sometimes.
Posted on October 6, 2005 03:42 PM | #
The thing about building a site these days is that if you’re building it from scratch and you’re smart with your templating, it’s just as easy to do a global search-and-replace for the word “blue” which appears inside a class attribute and change it if you want. I’m all for keeping your class names as semantic as possible, but there are just times when you want something blue for no semantic reason at all and that’s the only time I use colors in my classes.
In Christian’s example, I agree… it rarely happens… but you’re better off (albeit ever-so-slightly) by using names like “main-column” and “nav-column” etc if you can. But yeah, no big deal at all if you don’t.
Posted on October 6, 2005 04:35 PM | #
Very important subject Keith. If you work with other developers on projects for a long time, its always the best to comment something. Most Developers are gettin’ their own kind of codingstyle. Short & explicit descriptions are advisable.
Posted on October 7, 2005 01:13 AM | #
Here’s one that still gets me befuddled.
How do you layout your CSS file?
A lot of people go with the “new line for every property” layout, whilst I’ve always been of the “one line per selector” style.
Which is “best”??
Totally agree that comments are your friend and should be kept happy and content. Looking at some of the sites I have now and I still get confused, but then I’m no coder…
Posted on October 7, 2005 01:42 AM | #
Not to sound too arrogant, I think I can code HTML/CSS reasonably well. Coding nice with others is not my strong suite. I’m good at teaching and explaining approaches to a project. However, I kinda suck at documenting my own work and don’t always use a logical order for my selectors. Although I do write ‘margin > border > padding” in a fix order to quickly overview the rules on a single line. I don’t write my rules one below the other. Which doesn’t fit in the ‘playing nice with others’ theme either. Also I don’t always have time to ‘clean up’ dead or old code. Which adds to the confusion. Time I start to get the hang of this team thing.
Posted on October 7, 2005 02:11 AM | #
Small error:
“I don’t write my rules one below the other”
eh. with ‘rules’, I meant to say ‘properties’.
Personally I do beleive that declarations should be on a single line with it’s properties in a logical order. The exact logical order always seem debatable at best by the way.
My reasoning for having declarations on one line is that I think that keeping an eye on my selectors and their specificity is more important than the properties they hold.
I do have rules run across multiple lines (selectors comma seperated) but keep the declaration on a single line.
Posted on October 7, 2005 02:34 AM | #
CSS Files to me are a real problem. I seem to only like them one way and I’m not flexible at all. For one or two:
h1 { font-weight:bold; }
h2 { font-weight:bold; color:#FFF; }
but more:
h3 {
text-align:center;
margin:0 auto;
padding:12px;
font-weight:bold; }
And that seems to be the only way my mind likes to read it.
Posted on October 7, 2005 02:56 AM | #
Totally agree, having worked for 2 web agencies and now a much larger company, working with OPC is something you have to get used to.
The first web company tried to use standards and as such was generally good to code with but a lot of the other code from the next 2 companies is appalling meaning you can either just amend appalling code or rewrite and hope for the best!
Posted on October 7, 2005 06:48 AM | #
I agree 100% that less mark-up is better. I took over a Flash project developed by a .NET programmer with zero Actionscript experience. I was able to remove over half the original code, and noticed the application sped up a bit after doing so. If the previous author had bothered to comment his code, it would’ve taken at least a week off this project.
Posted on October 7, 2005 08:13 AM | #
I’ve just taken over a very large intranet that has legacies from two different developers with two different coding styles. I wish I my biggest concern was the naming of CSS selectors! Right now I’m up to my eyeballs in hundreds of directories on several servers (for no sure reason I can detect). The use of editors and display were clearly an afterthought, which is why I’ve had to figure out how to qickly sanatize (to some degree) pages with FrontPage, Dreamweaver and yes, even Word 2003. What I’ve learned from this so far (1st week) - at the very least, try to use the same sort of method for display site-wide, (global.css would be a good starting point) and please, utilize a reasonable, logical, hierarchical(!?!) directory structure! Not every file needs its own directory.
Posted on October 7, 2005 09:02 AM | #
Your lucky that you got to rebuild your system… I worked for 9 months on a customer’s site which was, for the most part, a clusterf*%k… An attempt at classes gone horribly wrong, display code mixed with logic code, and changing the display ACTUALLY affected the LOGIC!
Ive never seen anything like it.
Once a simple markup of HTML took 4 solid days to fix because changing one word of the HTML affected the PHP logic 2900 lines below…
After being promised the redesign, the customer went with another developer because, QUOTE:
“It takes you so long to update the site, I don’t want to wait that long for a redesign from you… But I would still like you to maintain it.”
So Even though the client was paying me $75 / hour, it wasn’t worth beating my head up against the wall to figure out that site…
Be very careful when entering OPC.
Posted on October 7, 2005 03:41 PM | #
The CRUCIAL thing to remember about OPC is that TCYWAMA (The Code You A Month Ago) might end up being your very own OPC ;-)
There’s nothing worse than looking at your own code a month later and saying out loud “WTF was I thinking?”
Comment like you expect to read your own code three project later…
WIHCFAMAITP (Wonders If He Can Fit Any More Acronyms Into This Post)
LOL
Posted on October 8, 2005 10:26 AM | #
* TCYWAMA (That Code You Wrote A Month Ago)
Posted on October 8, 2005 10:28 AM | #
The worst thing in the world is to get used to my supervisor’s codes. He is also a developer, but with very dirty coding habits. Bad logic, messy alignments, few comments, never debugging, never cleaning trashy and unnecessary codes. It was very hard for me to recognize the miserable logic behind his codes. And what is a real nightmare? He insisted me to code as same style as his! And he never pay attention to my words on how to improve the logic. If you look at our database, over 30% of them are useless and is stuffed with many different naming standards because he changes his mind pretty often although not hear to others. When his new standard comes out, he only wants this new standard applying to new codes without touching anything with old standards. I have been with such a stupid supervisor for two years. Not sure how longer I could withstand.
Posted on October 8, 2005 11:01 AM | #
The most difficult case normally for me is the arrangement of the CSS structure coding. There are some who build their markup according to presentation then the content controls. Normally, I prefer building my structure according to the website layout it. Like Header > Body > Footer so when you scroll it’s according to the section.
Posted on October 9, 2005 07:34 PM | #
Lately I’ve been coming upon a lot of ‘newer-code’ with classes like ‘tableHeader’ and ‘articleHeader’. Taking advantage of the tags that we are provided and not recreating them with classes and id is another important point to make. Why make a make a class=”quote” when we have the q and blockquote tags?
Posted on October 9, 2005 07:54 PM | #
Great post, Keith. As someone else mentioned, it’s been a long standing goal of mine ever since I started developing, that the next person to pick up my code ought be able to acquaint themselves with it fairly quickly.
For me it comes down to a couple basics:
- Keep your code concise, and well documented.
- Use common, documented best practices whenever possible.
And actually, I think those two recommendations go for any aspect of a project. From using proper UML and other business/functional requirements best practices, to making sure you properly document the I/A, etc. I hate coming onto poorly documented projects, and will do pretty much anything I can to prevent it from happening on any project I’m on from the beginning.
Posted on October 10, 2005 03:32 PM | #
Ok, so we don’t use class=’blue’. But I generally like to use quick reference rules like .c for text-align:center or .cl for clear:left; or .h for display:none;
I do think that there can be a happy medium on what’s super-semantic and what conveys just a tad bit of style. Because really, what’s one to do when you need to call out a (very) particular piece of text that isn’t going to happen anywhere else or just a few places. You might even be able to get away dropping the occassional style attribute. Granted, you have to question yourself when you’re dropping them left and right on every page of your site… especially for bloggers, it gets interesting when you want to call out a specific style right in the middle of a blog post.
Good thoughts.
Posted on October 11, 2005 10:08 PM | #
Writing CSS gets more and more complex, just like programm code. So designers can now learn from the coders how to treat such things or at least how not to do it.
A good thumb rule: Programs should be written for humans and only coincidentally executable by computers.
This can be applied to CSS as well.
Posted on November 23, 2005 12:04 AM | #
is a writer, designer, etc. in Seattle, Washington.
Home | Search | Archives | Subscribe
SOW: Say It Ain't So by The Jeff Lash Trio
Movable Type, Static Pages, Flexibility and Scalability
The highly recommended Dreamhost!