If the code works and I haven't written the same exact thing myself, the most negative comment I am willing to make is "complicated." At my advanced age, it takes nothing short of a critical system requirement that a piece of code can't possibly meet to rewrite something that works.
The jwz-documented Netscape rewrite debacle is brought to mind by the original posting.
Couldn't agree more! Most of the time I've heard people say "this code sucks" it seems to mean that it's formatted "wrongly" or that the person is too lazy to understand it, or it's not written by them!
Oh, I don't know. When I've encountered code that I say sucks, it was for code that had single letters as variables in massive function, didn't believe in breaking logic into discrete functions (this was primarily in VB/ASP, so it wasn't for perf reasons ;-)), used copy and paste all the time for large amounts of code (and since they didn't use discrete functions, lost out on simple abstractions), etc...
I agree, although I will mention that there are times when I've evaluated open source software and libraries by reading the code and if it looks like crap I'm going to be wary of introducing it into any of my production systems. In situations where you're really hoping that the library or utility is going to solve your problems as advertised the bias actually goes the other way. The code has to have some measurable level of suck to get rejected.
The Netscape example is an extreme case since it was an entire system rewrite (still a valuable example).
The other extreme of leaving the code to rot is just as bad. "It works... for now."
There are too many little changes that occur in either the larger system itself, the platform, or even the OS (and of course the customer) to just say "it works." This then leads to, "Well it should still work" as time goes on.
In my long history of development I've seen VERY few rewrites that were successful. I've seen probably tens that ended up wasting a lot of money and time and probably about two or three that proved to be worth it.
I really think the problem is that most devs don't know how to read code. Sure there are some programs that have pathological problems in them, but those are relatively rare. It is often just design choices or programming patterns that a particular dev doesn't like or understand.
Agreed. Having a mostly functional system with an occasional bug is certainly better then a half implemented untested version.
There does exist a point at which a rewrite is necessary and is indeed far less often than most would like to think.
I was dealing with some CSS recently that was just odd. Everything (yes, every single block element) was a float resulting in a soup of fixes and workarounds because of the oddities they introduced earlier. In rewriting it, I managed to lose more than half of the CSS statements and make it be an almost pixel perfect match and be more browser compatible.
It was written by another contractor and I was contracting. Handling the politics delicately was the real problem.
But I've seen code that really does suck, too, and I still had to understand all of it to really know the code is solving the wrong problem or alternatively not solving any problem at all.
She makes a good point, but sometimes, code does suck.
Personally, my worst experiences have been inheriting software written by kids just out of school who have read a lot of books on design patterns. On the bright side, I've learned some really interesting things from these guys here and there, they are typically really, really bright and extremely well read.
I've had instances where I rewrote core code from these type of guys and gotten it down to 30% of its former size, much more understandable, much faster, and much more flexible. But if we went into a meeting to discuss which is the better way, they would likely blow me out of the water (despite public failure) because they are young, inexperienced, and supremely confident.
I think maturity of attitude in software developers typically doesn't set in until 10+ years on the job. So many of the young people entering the field are all educated on the new "yet another revolution" practices (TDD, Agile, etc), and they are so confident, because they are smart and educated (which they are) that it makes interacting with them almost impossible. I've dealt with junior and intermediate developers that seem to think its impossible they could be wrong, they will cling to an idea even though you can easily give several examples where their theory breaks down.
I guess this is just the nature of our business, it is a bit too abstract, and education can be too easily mistaken for knowledge.
This is why we put comments there. Personally, I put comments so future me will remember what the heck I was thinking when I wrote it.
That said, there's no doubt there's code that definitely sucks. Some of it is mine, too. If I wrote it last year, it sucks. If I wrote it yesterday, it's really great. You can really see how much I've learned in a year when you compare it with my previous sample. At least if I have to rewrite the old code, I'll know what traps and special scenarios I need to watch for if I properly commented it.
I believe it's a good thing when I see code I wrote 6 months ago and think it sucks. If I thought it was perfect then that would be a sign that I hadn't learned much since I wrote it.
This could be a good metric to see if you are actually improving or if you are jumping the gun on labeling code sucky.
A lot of people think code sucks if they simply can't understand it at a glance. Of course you think yesterday's code is awesome, you just worked on it yesterday. But in 6 months, with the issue completely out of your mind, even if you haven't advanced, you might think it sucks.
> I put comments so future me will remember what the heck I was thinking when I wrote it.
People who don't do this: please don't underestimate the importance of this.
Be descriptive as possible with the comments on possibly clever/incomplete code - you will thank yourself months later when you realise:
x += 1 # border
makes no sense to you.
Yes, at the time you wrote the code, it's perfectly obvious to you, but you will forget it and think to yourself next time: 'wtf is this being incremented by 1 for, what about the border, is the border being incremented?'
If you hesitate even for a second about the possibility of your code being misinterpreted in the future, either fix it on the spot or add a damn descriptive comment.
You can drown code in comments, though. In your example, saying "x += borderW" would be as expressive, and tie that context to everywhere else it was relevant. Some comment issues are really better handled as (variable/function/method/structural) naming issues.
Comments are better for explaining the rationale behind seemingly arbitrary or crazy design decisions - "I would have done obvious thing X, but that causes huge problem Y". Why, rather than what. For things that provide important context, but could not be expressed in the language itself.
Many "what" comments should instead be static typing declarations, asserts, or tests. At least that way, people will notice when they no longer hold. Comments are just declarations the language ignores. (Citations, like "implements algorithm X, see Okasaki 1999", are fine though.)
If I see a piece of code and I think "this code sucks", go back to rewrite it and then realize mid-way that there was a reason they did it that way, the code does suck.
Good programmers leave comments saying "this is ugly because X".
I was working on updating a backup script, and it had the same function written ten times, with no arguments, entirely for the purpose of calling it. No closures, just pointless repetition.
Also, about half the lines of the script was other very similar blocks of code that had been commented out.
You see, when you say something like this, it would never occur to me that people can do this.
When I hear about bad code, I'm thinking about code that is not scalable or unmaintainable. Or, it doesn't follow a proper design pattern or maybe trying to be too clever coding. Or, maybe have resource leaking. Somethings just would never cross my mind.
Browse http://thedailywtf.com/ occasionally. While most of their entries aren't particularly interesting, there are some that will really educate you about human stupidity. Another interesting site, but that doesn't update much is Computer Stupidities, http://www.rinkworks.com/stupid/ , though they focus more on user denseness than developer screwups.
Writing code is easy, reading and understanding someone elses code is hard. At least, initially.
'Your Code Sucks' can be shorthand for 'not-invented-here', 'I'm not able to grok your datastructure' or 'I don't have experience enough to take this all in but if I scrap it and rewrite it it will be on my level' as frequently (or even more frequently) than that it really means your code sucks.
I'll say 'my code sucks' easily (especially the morning after), but your code might need some incremental improvement until I understand it thoroughly enough to make a judgement call. And in the interest of diplomacy I'd probably give you a hand refactoring it instead of passing judgement. I might even learn something.
I don't disagree that often I fail to understand why someone did something a certain way, on the first reading. But after I understand all the logic, and have run test cases etc., the fact that code is poorly structured, buggy, full of duplication, excess global variables, etc. is sometimes unavoidable. When that happens, I have to conclude that it does indeed suck.
I had someone contact me about a project they felt was taking too long. They had hired a contractor to do a Rails app, and wanted a second opinion the code to see what the developer was up to.
I got a copy of the code, and it all looked like boilerplate Rails stuff, pretty much by-the-book generated stuff. A first thought was, well, how freakn' long could it take to produce this? But it occurred to me that when you look at code (good or bad), there's often no way to know how the author got there. How much code was scrapped? What ideas were explored? Even when you have the repo history there's still no way to know what might be missing.
It's hard to look at code and make any judgments about the developer, since there are often numerous details that informed the decisions made along the way.
My favorite variant of this phenomenon is the time-honored tradition in consulting of the new guy throwing the old guy under the bus.
"I've had a chance to look through the existing codebase, and I'm sad to report that it's in pretty bad shape. I'm afraid we're going to need to rewrite a bunch of it before we can even move forward."
Wait, some code really does suck. The difficulty here is that "sucks" is a "catch all" that can include a lot of types of flaws. These might be flexibility, error-proneness, security, performance or maintainability/confusion.
In some cases the suckiness is an appreciated trade-off against time-taken and is bearable in maintenance. If this is the case we should try to be more diplomatic about the problems when assessing the code.
However, some code does suck. There is a certain type of easily preventable badness that slowly manifests itself in a project that isn't being effectively maintained.
Case in point in my current world of "joy":
public class MainForm
{
public static Form FormInstance;
public MainForm()
{
FormInstance = this;
}
}
If you're wondering, there are over 200 references in the code to MainForm.FormInstance.
If your goal was to prove that some code unambiguously "sucks", then your example works against you. Because as I read this I see what may be a perfectly reasonable solution to common problem. (For future-proofing, the developer wanted to be sure most of the code shared a single INSTANCE rather than a CLASS with all-static methods. Then later, multiple instances could be used if needed. But since for now only one is used, she took the quickest and easiest approach to instantiating that single instance: no need for fancy factory classes when the instantiation will only be done once!)
This is a failed attempt at a singleton pattern. The main problem here (there are MANY) is that there is no check to see if a Singleton already exists, meaning that the "Singleton" can be scrapped and replaced with a new object (most likely by accident) at any time.
she was saying that at that point she was not ripping out huge swaths of code and rewriting them, 'decimate' actually makes plenty of sense in this context
There is a difference between code which disagrees with your preferred conventions, code which is ugly but right, code with serious architectural flaws but which is good enough for right now, and bad code which has bred until it is an inescapable tar pit of unnecessary complexity.
The ability to tell the difference comes with experience, as does the ability to abide the first three.
However, I have found that experience has changed my approach to the last away from "complaining" and toward "fire and brimstone sermon".
When I was younger and more naive I used to think almost every code I ever saw sucked, but especially if it had any of the following characteristics:
- Indented in another way than I preferred to or the format was in some way not the way I do it (camelCaps, spaces around parenthesis.
- The code was doing to much in a single line. Preventing readability, etc.
- It had inline HTML or just anything less than 100% separation of code and presentation.
- I didn't understand it
These days I almost never think anything sucks, even though it's written by some beginner right out of grad school. However there are exceptions and now the things that make my dislike some code might have something like:
- Big huge functions that do too much without splitting it up into smaller chunks
- Things done in many lines of code that can be done in a single line [in a simpler/faster way].
- Obvious slowdowns, such as way too many SQL queries than needed, for loops that go through 10,000 cycles when you can get the same result in 15 cycles, etc.
I'm guessing in the future it will be something else entirely that I will look at and vomit. And it will most probably be my own code.
IF the client comes back and says "We're dissatisfied with your product" - that's one thing.
But "I showed that code we paid you for to someone else and they said it sucked" is unprofessional and pointless.
The only response I can think of was "That's interesting - could you perhaps give me some feedback as to what ways you were dissatisfied with the app?"
It's a great marketing strategy for the second-opinion consultant, though. I actually have been in this exact position. I entered into an ill-concieved "startup" project with someone who I considered a friend. It was a specialized e-commerce app that involved interfacing with some proprietary mainframe systems I had never worked with before. He thought I was taking too long so he hired some random guy from the internet to review my code. The guy said my code sucked... then turned around and said he would fix it, for about triple what I was charging. This sort of ended the business relationship (and friendship) between myself and the other founder. My old partner ended up hiring the other guy and paying the 3x rate, and the guy didn't even work on the hard part, interfacing with the mainframe! It ended poorly for me, but I learned a lot about how to market yourself as a consultant from the other guy: charge a lot, say the working code sucks and needs to be rewritten, avoid doing anything with a large technical risk, etc.
I rarely find myself thinking "this code sucks." Usually it's "this code is typical." Sometimes, it's "this code was designed to solve a problem we no longer have and gets in our way solving the new problem."
Either way, sometimes I do in fact see code ('not-invented-here') that clearly does not suck. It's concise, modular, comprehensive, and easy to trace and modify. There's a consistent and seemingly effortless balance abstract and domain-specific functions and logic. There might be jokes and humor in the code, but nothing that ever gets in the way of understanding. Nothing like "hmm I don't really know what to call this temporary variable so I'll make up a funny name for it"
The whole world sorta sucks; I have seen very little that could not have been done better. Also, this is the reason professional writers have someone else edit their writing or, where that isn't possible, set it aside for at least a week, longer is better, before coming back and editing themselves. Coders are just arrogant if they think they should be able to write good code the first sitting. Of course, unrealistic schedules forced on them by management has a lot to do with that in many businesses.
One should always hold back un-constructive jabs, but some patterns of suck (ex: c&p vs looping) can be detected and undone even without understanding the entire project.
That said, I sympathize with anyone who is doing iterative client work on an hourly basis, and is called out on re factoring. Explaining the cost and benefit of refactoring work to clients can be a hard upsell, and IMHO helps separate great clients from not-so-great.
As a general rule of thumb my unease with a line of code is proportional to its distance from the left margin. There are exceptions, naturally, but that's generally how I judge myself: if I open a file and see 16 or 20 spaces on a line I'm probably Doing It Wrong™.
I've seen some really bad code... it didn't even work properly! I think that should be the benchmark for sucking. Once it does work, I don't think it can 'suck.' It can be inefficient, it can be slow, it can be improved, but let's reserve suck for real bad code.
It's much more difficult to read code than to write it. Even when using the same coding style and conventions, looking at a problem that some else solved will look confusing to you. It's easier to say that it sucks than to say you don't understand it.
I get the premise of the article, but I think that you can quickly look at some code and determine if it passes the sniff test. I've been guilty of rewriting sections of code just b/c I didn't like the formatting and other minor annoyances, but then again, there is just general newbie or outsourced lazy, sloppy, spaghetti code. You can spot that in less than ten seconds and it truly does suck. Engineering is about integrating prior solutions to solve a problem. Many times that means the experience to know I can pull in a library and write 10 lines of code to solve a problem instead of hundreds....of bad lines.
There is no such thing as code sucking, only code that works and code that does not. If someone said that to me, I'd ask the client how much they wanted to pay me to make it please who ever they paid to review it. A bill usually shuts down inane comments quite quickly. The folly of their opinion is revealed when they don't want to pay to "fix" it because there is no problem.
While I'll be the first to admit that many (perhaps most?) developers (present company included) have a tendency to quickly conclude that other people's code sucks (at least they do at some point in their careers), I have to disagree with the idea that there is no such thing as code that genuinely sucks.
I've read a lot of code in my lifetime, and some of it genuinely sucks. There is a such thing as a bad developer. There are people doing programming who really have no talent for it, or for some other reason are just really bad at it. Sometimes it is due to lack of experience. Sometimes it is due to the person not caring at all. And I am sure there are plenty of other reasons. It doesn't mean that a person can't get better, often they can get much better, but people do write code that sucks.
Understand, I think I am now much slower to be harsh toward other people's code than I used to be. In fact, I regularly give people the benefit of the doubt. Why? Because I've worked for that megalomaniac that wants done yesterday what can't possibly be done in a month's time. You know, the one that thinks that because his work consumes his entire life, and causes him to neglect his family and other responsibilities, so should yours. I've worked in multiple high-pressure situations, where no one cares what the code looks like, as long as it is done tomorrow. And you know what? Sometimes in situations like that people slap together whatever they can (as fast as they can) with duct tape and chewing gum, whatever it takes to get that unreasonable person (or people) off of their back. The results are often ugly, and you can argue whether the person could have done it better in the same (or even less) time. But I think it is worth cutting the other guy some slack. Thankfully, I no longer work for the megalomaniac (and haven't for many years) :-D
However, getting back to my original point, there is code that sucks. It may not be as prevalent as some think that it is, but believe me, it is out there (even after giving the other guy the benefit of the doubt).
Geez, what's up with me and the parenthetical kick today (perhaps I've been writing too much Scheme code recently). :-) <- got one last one in there.
There is another reason code sucks though, besides being over complicated: Redundant logic and/or not abstracted enough. Sometimes the person was just getting things done and abstracting the logic for reusability wouldn't be necessary in the long run because it was a prototype etc... However, the vast majority of the time I find that taking out redundancy in my code speeds up development not only later on but for the initial release.
I say "this code sucks" pretty often. Occasionally, I'm wrong in ways similar to those described in this article. Often, it's crappy outsourced code built by intern developers with no sense of right and wrong.
Sucks is a little strong, but a single CSS edit it would make it much easier to read.
Notice how your navigation elements are 5px larger than the body text. But when we think about it, the primary use of a blog is for people to read the content - not navigate around. I suggest bumping the body p's font up from 12px to 15-16px.
Here are two screenshots, one of the default rendering in Safari and one with the Reader feature turned on. Notice how much larger the reader font is than what's specified on the site.
It is a hook, a clever bit of marketing oneself and I haven't got a problem with that.
I have an interest in a female peer's perspective on our industry. I have yet to work with a female coder and feel my working environment would be improved with a better male to female ratio.
Personally I think it's smart. I'm sure she gets more page views because of it as well. First time reading the blog and I thought it was interesting enough.
I don't see how she's intentionally fooling you. The URL is girldeveloper, she has a picture plastered on the right (like a ton of other blogs I might add). I guess I'm confused by your reaction.
If all she wants to do is get page views, then she's succeeding.
By positioning herself as a "girl", I think of her as a girl first, and a developer second. What qualities do I associate with girls? Not the qualities I look for in a programming blogger. Is that prejudice? I guess so.
I clicked the link because it had a catchy title, not because it was from a site "girldeveloper". That I'm not even thinking about the content of the post anymore speaks volumes.
Women are a significant minority in software development. This in itself means that a woman's experience of the field I work in every day must be different to mine. You're more likely to learn something new from someone looking at the same things you do from a different perspective than you are from someone who shares your POV and is more likely to just tell you things you already know.
The blog still has to be interesting, but "girldeveloper" is as good a hook as any.
Where is your blog? I mean common, I never see mean comments like these in hacker news posts. Attack her ideas, totally fine with me, but leave the petty comments at home.
Even if she did does it matter what some random person on the internet says? Really?
On the other hand if she did improve her text color a bit then it would be awesome. I loved her theme though. It's quite pretty in a girly girl sort of way.
Interestingly, the other way around is pretty good. That's why road and highway signs are white on green, it's the third most readable combination after white on red (#1) and black on yellow (#2).
I never thought about why road signs are like that, but you are right. Interesting. Unlike my observation about the website, which somebody finds offensive or wrong, it seems :-D
The jwz-documented Netscape rewrite debacle is brought to mind by the original posting.