Updates from Joel Toggle Comment Threads | Keyboard Shortcuts

  • Joel 10:56 PM on January 11, 2017 Permalink | Reply  

    Code Smells 

    Some things in life stink.  Literally.  Just ask any parent with a small child who is still in diapers.  When something stinks, it’s natures way of telling us that either something needs further investigation (like a stinky diaper) or it’s a sign that we should stay far, far away (like a rotting corpse).

    You may have heard someone say that particular code doesn’t pass the “sniff test”, or that the code “stinks”.  To the inexperienced, it can be difficult to detect this.  What are some indicators that code stinks?

    Variables that are declared, but never used.
    If it’s there but not needed, why is it there?

    Poorly named functions or variables.  If you ever see something like “someFunc() someFunc2() and someFunc3()”, that’s not a good sign.  It’s a sign that code was either written in haste or not properly cleaned up after the fact.

    Large swaths of commented out code.
     This indicates that code isn’t being properly cleaned up.  If you need to do something drastically different, do it in a different source control branch.

    No code comments at all.
     I recently heard of a rather controlling developer that insisted that there be no comments in code at all.  While I believe that code should, to a certain extent, be self-documenting, this is an extreme.

    Lots of “to-do” comments.
      This indicates that things aren’t done.  This is probably fine if you are a single developer working on a single project, but multiple developers working in the same code should probably be using something better to track what still needs to be d one.

    Functions with “or” in the name.
     (e.g.: GetCountryOrState()).  This is a sign that things should be broken down into simpler steps.  For example, it could be split into a GetLocation() function which could then call GetCountry() or GetState().

    Constant variables that aren’t explained.  There’s nothing quite like wondering what is so special about the number “35.10235” without anything describing what it is, or where it came from.

    Excessive amounts of string parsing.  Unless you are writing something that handles a ton of  string related data, and you have to do some seriously funky things to it, excessive string parsing is a bad smell.  Most of the time it’s not necessary, or there’s a better way of doing things.

    In Python, deepcopy().  Sometimes it is a necessary evil, but most of the time it isn’t.  If someone is using deepcopy(), especially if they use it a lot, that’s worrisome.

    In any language, using the equivalent of eval().  eval() basically takes in a string, converts it into code, and runs that code.  Any time you use an eval(), a hacker grins.  I’m sure there’s a valid use case for turning a string into compiled code, and then running that code, but I have yet to see one – other than a compiler.  This kind of stuff is especially dangerous with JavaScript.  If you are using eval() in a web page, you’re just begging to have hackers abuse your system.

    Not using stored procedures when you could be.  If you are ever working with a database, you’d better be using stored procedures.  I’ve seen my share of horrible things, including this.  Stored procedures won’t protect you from everything, but if you are still open to SQL injection attacks because you aren’t using them, then shame on you.  One particular application I worked on had SQL queries happening in button even handlers.  The original developer never accounted for people like Mr. O’Conner, who happens to have a single quote in his name.  A malicious user of an application like that could have easily destroyed the entire app’s database with a single, well placed string.  It’s 2017 – there’s no reason why anyone should be combining strings to make queries when stored procedures are an option.

    I’m sure there’s lots of other code smells, but these are the ones that stand out to me.

     
  • Joel 10:45 PM on January 4, 2017 Permalink | Reply  

    Where does your time go? 

    For Christmas, my wife got me this book:

    soviet_ghosts

    It’s a book of photography of abandoned buildings from the Soviet era.  The one thought that keeps going through my mind is how many man hours were spent building such amazing structures, only to have them decay into nothing but waste.

    But then I keep remembering that I’m a programmer, and how little of my code will likely still be in production in 10 years.  It’s essentially the same thing.  All my work will eventually be tossed out, abandoned, and left to rot.

    It’s a bit of a strange feeling.  But then again – the most important things in life aren’t things.  Time spent on ‘things’ can be good, but time spent with people is worth so much better.

     
  • Joel 10:46 PM on December 30, 2016 Permalink | Reply  

    Bad Developer No Twinkie Part 9: Not asking for clear specifications 

    As a software developer, it is your job to build the right thing.  If you don’t know what the right thing is, and you’ve already started coding, you’re doing it wrong.*

    Before you even consider opening your favourite IDE,  you need to know what you are building.  Part of this is dealing with other people, and getting clear specifications on what it is you are building.  If you fail to get clear specifications, it’s your own fault.

    Specifications can be as simple as:

    • It must allow a user to select x, y, or z.
    • It must run in under 10 seconds.
    • It must work for multiple users at the same time.

    This seems like one of the most basic tenants of developing anything, but we still do this far too often.  You don’t see a construction worker start by ordering a load of lumber or bricks unless he’s got the blueprints in hand.

    • The exception being when you are mucking around and prototyping stuff.  It’s fine to play around with code for your own sake.
     
  • Joel 10:22 PM on December 28, 2016 Permalink | Reply  

    Bad Developer No Twinkie Part 8: Not paying attention to performance 

    Those of you who do graphics programming for games already know this.  Move along  The rest of you: feel free to pay attention.

    You need to pay attention to performance.*

    Numerous times, I’ve seen someone say something along the lines of ‘Well, it worked well enough with 10 records.  I didn’t test it with 100,000.’  Far too often we don’t pay any attention to performance, and it comes around to bite us in the butt.  We test with small data sets, and we expect the same performance when working with 10x or 100x as much data.

    Thinking about a fairly recent project I worked on, a particular part of the website worked fine for testing purposes with a few records.  When given 20 or so records, a particular feature of the site slowed right down, and became somewhat frustrating to the users.  Rather than being able to immediately click on a record to see the details of that record, the user would instead have to wait several seconds – even after the initial list of records was displayed – before a particular UI element became enabled (which allows the user to see the details of the record).  This was in production.  How did it get into production with terrible performance?

    1. It was rushed, and never tested properly.
    2. It was never tested with a data set larger than 5 records.
    3. It suffered from poor communication between the UI team and the back-end team.

    * Except When You Don’t.

    Like anything else, there are exceptions to this.  Premature optimization is the root of all evil.  Remember the order:

    1. Make it.
    2. Make it work.
    3. Make it work right.
    4. Make it work right, fast.

    There’s a difference between premature optimization and being downright sloppy and not planning for the future. Saving a single allocation in a for loop is premature optimization. Knowing that a particular feature is going to have to run “fast” on 100,000 pieces of data is a whole other thing. This is where it helps to have performance hammered out as part of the requirements. “Needs to do x in less than y seconds” makes it possible to say “Yes, this passes” or “No, this is too slow”.

    I always wonder about the guys who write installers.  When running an installer, it seems like half the time my machine isn’t taxed by CPU, memory, disk usage, or bandwidth limitations.  If it isn’t doing any of those things, what the heck is it doing?  Where’s the bottleneck?  I understand that having a fast installer isn’t a high priority for a lot of people, but if it’s your job to write installers, it ought to be a priority.  You’ve only done 3 out of 4 of the above list.

     
  • Joel 10:05 PM on December 22, 2016 Permalink | Reply  

    I’d like to buy a vowel… 

    Back in the day, variable names were limited.  You might only have 8 characters to describe whatever piece of data it is you are working with.  Thankfully, that’s no longer the case, and it hasn’t been for some time.  You shouldn’t ever be afraid of using variable names or function names that explicitly state what you are doing.  Naming your function “fun()” isn’t going to help anyone.  Naming it “computeCentroid()” will.  I really don’t see a lot of benefit in shortening single words, like using “desc” instead of “description”, unless variable and function names are already getting long.  In such a case, pick one, and be consistent with it.

    At the same time, don’t be too wordy.  When first starting into a project, which involved converting an old Microsoft Access and Visual Basic project into a Django based web service, we were dealing with a name like “Sub-terrain Frobnicating Zone”.  Because we weren’t familiar with its abbreviation (STFZ), we were naming variables like “localSubTerrainFrobnicatingZone” or “heliocentricSubterrainFrobnicatingZone”.  This got pretty old, pretty quick.  After working with the project for a little bit, we quickly adapted the abbreviation of STFZ.  So the above variable names would become localSTFZ or heliocentricSTFZ.  Much more manageable, but still descriptive, assuming you knew what an STFZ was (which was covered in the project readme).

    The beauty of working with good tools and statically typed languages is that your tools can tell you what type something is if you mouse over it.  For example, if the above code was C#, and I was working in Visual Studio, I would only need to mouse over a variable name to see what type it was.  So there’s something to be said about naming a function “ComputeCentroid()”, and being able to see what it takes for parameters and what it returns, rather than naming it “ComputeSubterrainFrobnicatingZoneCentroid()”.

    tl;dr – Be concise and consistent in variable and function names.

     
  • Joel 11:19 PM on December 21, 2016 Permalink | Reply  

    Code Rot, Technical Debt, and Refactoritis 

    Code rot.  Technical debt.  Call it what you want – it’s essentially the same thing.  Over time, code becomes more and more of a mess, and developers want to work with it less and less.  If the code smells, no one is going to want to work with it.  That’s fair – no one wants to live in the house next to the sewage treatment plant.  But it doesn’t need to be that way.  Code doesn’t need to smell.

    Software development is unlike almost any other industry out there.  We have so much flexibility, yet we seem to so often fall into the same traps – again and again.  One of these traps is not spending an appropriate time on maintenance.  We write systems, then come back to them six months later, after something breaks.  Since then, everything has changed, and the original code no longer does what it is supposed to be doing.

    Imagine this: It’s the year 1916.  You’ve built a brand new house with the latest amenities – complete with indoor plumbing and heating!  Now keep the house the exact same, and fast forward 100 years.  Even if you managed to perfectly preserve the building materials, I don’t think too many people would want to live in such a house.  There’d be no electricity, no double-paned windows, no high efficiency furnace, and gasp – no internet!  Even if the house was in perfect condition, it no longer meets the needs of those using it.  Unless the occupants are also 100 years old, in which case you’ve probably got bigger problems.

    Code is like this.

    Even if a piece of code doesn’t change, everything else around it will inevitably change.  Technologies develop, related systems get upgrades, and business requirements change.  Rarely does something in software stay static for multiple years – let alone multiple decades (or even months!).  You need to occasionally make a few tweaks here and there, re-write the occasional function, update documentation, add additional unit tests, and so on.  You need to spend a little time doing some proper maintenance, and occasionally add a new feature here or there to make sure that a given system is meeting business requirements.  Routine maintenance is critical to keeping systems operational.

    The other extreme of a lack of maintenance is something I like to call “refactoritis”.  This is something a lot of developers suffer from.  Once a particular problem is well understood, and a system is written, a developer with refactoritis will want to immediately refactor it.  It’s a dangerous, slippery slope.  There’s nothing inherently wrong with refactoring code – just like there’s nothing inherently wrong with open heart surgery.  But there is a problem with it when it is done without reason.  It’s a difficult itch to scratch, and once you’ve refactored one thing, you’ll want to go on to another… and then another.  And before you know it, you’ve blown through all your time just re-writing the same few pieces of code again and again, and nothing actually gets done.  Client requirements still aren’t met.  People get angry.  Bad things happen.

    So how does one find balance between the two?  Experience.  Either your own or someone else’s experience can be amazingly helpful.  This includes guidance from more experienced developers.  They will know when to push back to get more time to do a bit more maintenance, but they will also recognize when it’s time to move on and work on other features.

    Don’t let your code rot.  But don’t catch refactoritis.

     
  • Joel 10:39 PM on December 2, 2016 Permalink | Reply  

    Bad Developer No Twinkie Part 7: Assuming data is good 

    Bad data happens.  Sometimes it happens accidentally.  Sometimes it happens intentionally.  There’s one thing that is for certain though – it will happen.

    This week we got hit by a particularly nasty bug.  It happened entirely because the way a particular piece of code was running – it made the assumption that an incoming piece of data was ordered by a given attribute.  In 99%+ of the cases, it was ordered, but no where in the specifications did it say that it would be ordered.  In the cases where data came in out of order, we were doing things incorrectly, and we generated bad results.

    I find this particularly true when working with Python and JSON data: never trust the format of data (especially the contents of Python dictionaries or JSON data).  For example, in python, you would do something like:

    someVal = someDictionary["level1"]["level2"]
    

    If someDictionary was loaded from a piece of JSON from a web request, how do you know that “level1” is there?  You don’t.  In such a case, the above line will explode.

    I am quite fond of the new C# way of handling things with the null conditional operator:

    var someVal = someObject?.someProperty1?.someProperty2;
    

    In the above code, someVal will only take on the value of someProperty2 if someObject is not null and someProperty1 is not null.  One nice clean line.  You still have to check the result of someVal, as it could still be null, but it is a nice, clean way of doing things.

     
  • Joel 11:15 PM on November 29, 2016 Permalink | Reply  

    The problem with HTTP status codes 

    HTTP status codes are pretty useful.  They can tell you when things have gone wrong, and tell you roughly what went wrong.  There are, however, cases which don’t seem to be covered by them.

    In my job, I work with web services – both writing them and consuming them.  Our team, in particular, has a number of projects that provide a web API endpoint to the UI team, and we in-turn, rely on other web services.  Some of these web services are written by us, and some are not.

    When the user makes a request from the UI, it hits an endpoint somewhere in the bulk of our application.  From there, it may go on to call one of these other external services.  In general, if the user makes a bad request, we immediately return with an HTTP 400 (bad request), and we try to indicate that the user did something wrong.  If something terrible happens (like an unhandled exception), we kick back with an HTTP 500 (internal server error).  But what do we do if one of the external services that we rely on fails?

    It isn’t necessarily the user’s fault, as the user didn’t do anything wrong.  It also isn’t necessarily an internal server error, as it wasn’t our service that failed.  Not only that, but all the contents of internal server errors are replaced with a generic message, so as to not leak things like stack traces, which would make reverse-engineering things much easier.  So if we return an HTTP 400, it makes it look like the user did something wrong (which isn’t the case).  If we return an HTTP 500, we can’t tell exactly what went wrong.  In cases like this, we’ve been doing one of two things:

    • Return an HTTP 400 error anyway, so we can at least tell what is going on.  This is less than ideal, but at least allows us to see what went wrong, and the UI properly displays something to the user.

    – Return an HTTP 500, but with a specific content.  Rather than having the body of the request replaced with something generic, we fill it out with detail on what went wrong, and hope that the UI knows how to display this properly.

    Both still seem a bit less than ideal, but there doesn’t seem to be an HTTP status code for “Hey, something went wrong with an external service, even though you did everything correctly.”

     
  • Joel 10:39 PM on November 24, 2016 Permalink | Reply  

    Bad Developer No Twinkie Part 6: Don’t lie to your users 

    No one likes being lied to, even from software.

    Take the following screenshot for example:

    wp_ss_20161101_0001

    Do you think I’ve got no browsing history?  If you did, you are wrong.  See the little blue dots at the top of the image?  That means it’s still loading.  One part of the screen was telling me that had no browsing history, while the other part of the screen was telling me that it was still loading.  (And for whatever reason, loading a browsing history in Windows Phone 8.1 takes forever).  So basically, part of the app is lying to me.  Wouldn’t it make so much more sense if it said “Loading…” instead of “Now browsing history”?

    Don’t lie to your users.  If you don’t know something, indicate that you don’t know it.

    I’ve had Outlook do this same thing.  One part of Outlook was saying that I had an unread RSS feed, yet actually looking at the RSS feed items, there was nothing unread.

    Remember the wise words of Thumper (from Bambi) when it comes to UI:

    If you haven’t got anything truthful to display to your users, don’t display anything at all. falsified data.

     
  • Joel 6:57 PM on November 18, 2016 Permalink | Reply  

    Bad Developer No Twinkie Part 5: Assuming people know things 

    As a software developer, you have to work with people.  It’s part of your job.  Sometimes those people are other developers, sometimes they are your clients.  Despite what it looks like, software development is rarely, if ever, a solitary effort.

    As such, you should never assume that whoever it is you are working with knows everything.  I had two rather frustrating instances this week with two different parties, simply because they made the assumption that I knew things – things which were not obviously stated.  It essentially made me feel like an idiot.  No one likes feeling like an idiot.

    Let’s look at a few examples of this:

      • Don’t assume your clients know technical details.  Maybe you are lucky, and you have a tech-savvy client who actually knows a bit about software development.  That doesn’t mean they know all the ins and outs of whatever it is you’ve built for them.  Don’t treat them like an idiot, but also don’t assume that they know everything about the software you’ve built for them.
      • Don’t assume new developers know the technical details.  Building software is often a complicated affair.  Every piece of software is different.  So is almost every development shop.  New developers won’t know all the details of the stuff you’ve built.  Take the time to coach them properly.  Time invested now is more time saved later.
      • Don’t assume developers have all the domain knowledge they need.  Developers don’t know everything.  I’ve worked on software for at least two major different industries.  In both cases, it took a fair bit of reading and discussing with other developers on the team to gain the necessary domain knowledge to fully understand the problems we were trying to solve.  Even after working in a particular industry for a few years, there’s still a lot that I don’t know.

    Don’t take this to the extreme though.  Treating your customers like they don’t know anything has a similar effect.  Essentially, if you are doing your job right, you should be in frequent communication with your clients anyway, and should have an understanding of where their knowledge level is at.

     
c
Compose new post
j
Next post/Next comment
k
Previous post/Previous comment
r
Reply
e
Edit
o
Show/Hide comments
t
Go to top
l
Go to login
h
Show/Hide help
shift + esc
Cancel