The Tommy Code

Story Points vs Hours

2011-10-17T20:01:00.001+02:00

In the Agile methods many suggest using something called Story Points instead of time when estimating the time a task would require for implementation. Rather than saying that it will take two days they would say it takes, for example, four Story Points. They then track what is called Velocity to see how many Story Points the team can complete in a week and suggest that about that many Points, let's say 20, is what the team can implement in a week.

I find this very awkward. Is it just a technique to hide the hours? Because, after all, most of us are still payed by the hour, and we also want to have an idea about the dates we might expect things to be finished. Story Points take this away. What would you say to the carpenter who you pay by the hour if he said he will have your bathroom done in 80 Story Points? He expects to burn about 20 Story Points per week, he adds. I would think he was a freak!

The only reasonable reason I find to use Story Points instead of Hours (or Days) is the notion that different coders may need different amounts of time to solve a task. With Story Points two coders may estimate a task to, for instance, four, while using days one might need two days and the other three days. That would implicitly say that coders are linear to each other in skill difference, since for the equation to sum up you'd need them to agree that the next task, estimated to two Story Points, would take the former one day and the later one and a half. Something I strongly disbelieve.

The more unreasonable reason is that as estimates are hard to do, you put a metric on them that is hard to understand to get away with them easier. If you say that something will take three days it's easy to see if you were right, while estimating it will take three Story Points will keep you safe. Who are to say how long a Story Point is? Hardcore agilists laughs at such a silly question.

I guess I'm missing something, but I have a hard time seeing the Point of Story Points. Feel free to enlighten me using the comment field!

p.s. I've never tried Story Points, so I might be totally wrong. It may be Gods gift to software developers. d.s.

EPiServer 6.0 compare bug

2011-10-14T22:47:00.000+02:00

I've spent most of today, and a couple of hours yesterday, to figure out a weird problem with the function that allow you to compare two versions of a page in the EPiServer 6.0 edit mode. As soon as I clicked the compare button I got the YSOD telling me that a NullReferenceException was thrown from the method ComputeVaryCacheKey deep down in the ASP.NET rendering.

I hooked up the debugger and found that everything seemed to be in order. Altho, we had a few customized renderings in the chain so I started by disabling them one by one to find the cause. Once they were all removed I still had the problem tho, so I started on another track. Next thing I did was to look at the output caching used. Starting with simplifying the GetVaryByCustomString method in Global.asax so it should be almost impossible to have a problem there didn't help. Next thing was removing the OutputCache directives on my controls one by one and in the end - bingo! When the last OutputCache directive was removed I got some other error instead from the compare button.

So it comes down to that adding an OutputCache directive with VaryByCustom would make that ComputeVaryCacheKey crash when trying to compare two pages... I really need my output caching, but for the sake of coming to the bottom of this I left them out looking at the new error message. Maybe it could be the root course and if that was solved I could add the caching again?

The new error said "']]>' is not allowed within '<!CDATA[' blocks" or something like that. Well, fair enough. I looked at the source of the page being compared and there were a couple of CDATA blocks in the scripts generated by ASP.NET but none of them seemed to be malformed. At this point I decided to fire up another EPiServer site and see if the CDATA blocks looked the same, and if they would like being compared on that site. The CDATA blocks looked the same and they did like being compared in that context... Odd. Here I decided to add the OutputCache directive to a control on this site and see if it would work here - And it DID! There was a difference tho - this site was build on EPiServer 6 R2.

At this point I contacted the EPiServer support that within an hour reported back to me that this was a known bug in EPiServer 6.0 that had been solved in R2. The general advise was to upgrade to R2, but in case I couldn't do that I also was send a custom "CompareProvider" written by the support guy that should work with 6.0. So I put that in the bin directory, configured the site as he instructed me for activating this new provider and gave it a go. I still got that "']]>' is not allowed within '<!CDATA[' blocks"... but the good part was that I could add my OutputCache directives again and still getting that error!

Well, now at least I had my own custom code running closer to the problem. I reflected the provider that the support guy sent me and it was only one class. I copied the code to a class in my own project allowing me to set a breakpoint in there. When I had the code it was also obvious that the CDATA issue was addressed in it, but apparently it didn't address the problem well enough. I started by just removing the CDATA start and end block from the input to the compare function and then it worked! Or well, kind of. It didn't seem to have support for css being added with @import so I got a comparison of the page without styling... still, it was the best yet!

Hacking away even more I hardcoded an addition of the primary css to the source, and now it looked good too! A bit too hacky tho, for my taste, it struck me that I could reflect the source from the EPiServer 6 R2 binaries to see how they solved it there. The first line in that version was the Obsolete attribute - use "HtmlDiffProvider" instead it said. Wonder if that works with 6.0 I pondered, and reflected that source and added it to my project next to the code from the support guy. Then executed... and boom! That first NullReferenceException from the caching again! Dang. But, with the code from these two providers I managed to create a one provider that worked fine with EPiServer 6.0!

During this entire episode I did of course do a lot of searching on the web for guidance... getting none! Hence I decided to write this post to help the next person that enters this moat. It's not your fault! It's a bug in the platform! Stop wasting your time looking for problems in your project, and instead add your own CompareProvider.

If you can't reproduce it, you can't fix it

2011-02-24T12:47:00.003+01:00

Every now and then you will be assigned a bug that seems impossible. You try to reproduce it, but you fail. You look at the code, and what happened seems impossible. It may now be tempting to add some if-statement to just make sure the object isn't null (for example) and close the bug. Don't Do That!

Doing that will cause a lot more trouble down the road. The problem will most likely come up again in some other part of your code, and then you'll be on yet one unplanned rescue mission. Maybe you'll be tempted once again to add a quick-fix and before you know your code will be full of them - all added with your best intention.

Then come the day when you find the real bug! Hooray! Finally your days of applying that dirty fix on different spots around your projects is over. But will you go back and revert your qd:s? No. Either there is no time, or you don't want to touch what works, or you just leave them there to take care about if this would happen again - but if you are like most people you will surely leave them there to rot. Don't Do That!

Leaving them mean more code to maintain, more things that can go wrong and more useless processor cycles. If there is an "if" in your fix (and it usually is) there will also be more forks in the code, which is very error prone.

So what should you do? You should put all your effort into being able to reproduce the bug. If you still fail, ask someone to help you reproduce the bug. If it seems to be a timing issue, write something that pushes your code so much that you cause the error within reasonable time before you try to solve it. Once again, how can you be sure you've solved the problem if you can't test if the problem disappeared when you're done?

Toedipping in ASP.NET MVC

2010-12-21T17:20:00.000+01:00

Today I've finally had time to have a closer look at ASP.NET MVC, getting my hands dirty and actually doing something with it. I have vacation and the kids are at day care - so this was my chance!

I started up downloading the Visual Web Developer Express 2010 and created an ASP.NET MVC 2 template project. I executed it on the development web server, saw it worked, changed a couple of texts and reloaded to see my changes were applied. They were!

Then I googled to see if my web host, Binero, had support for ASP.NET MVC at all. I found that they had a blog entry with an instructional movie showing exactly how to make it work, so it shouldn't be a problem I figured.

I followed the instructions and uploaded my modified template site. It turned out the template site is using .NET 4.0 though, which Binero doesn't support, so it didn't work out of the box. Changing target version for the project to .NET 3.5 should do the trick I figured, so I did that and tried to run the project - and got some strange error. The page said Error CS1525 about a line in the View where the code was something about "Html.ActionLink". Google didn't quite help me, but pretty soon I noticed the obvious problem. It was marked "<%:" which is new in ASP.NET 4. Changing all "<%:" in the views to "<%=" made it work with the .NET 3.5 framework and I was back in game!

Playing around adding my own models, views and controllers was all pretty straight forward. Nowhere near as much magic as I had expected. I got a hold of it pretty fast and only had one thing stopping me so much I had to go googling again. Adding " - MySite" after the ContentPlaceHolder inside the title-element was not doing the trick for some reason.

<title><asp:ContentPlaceHolder ID="TitleContent" runat="server" /> - MySite</title>

All I got in the title was the content from the placeholder... I first thought it was some update problem and started recompiling, reloading, refreshing, re-everything, but no luck. Finally I went to google who could help me instantly. It has to do with the head-tag having runat="server" set. Full description and solution can be found here: TipJar: Title Tags and Master Pages

A couple of hours resulted in a very simple web site, but never the less a ASP.NET MVC web site.

My first!

How to commit code

2010-11-18T13:09:00.003+01:00

When you commit (check in) your code to the source control system there are some things you should do to ensure quality and trackability.

1) If your source control system supports change sets, that is commiting a set of files as a bundle, make sure you just include one issue in that bunch. You shouldn't fix a bunch of things in all ends of the project and then commit them all in the same change set. The change set may well span over multiple projects tho, because one change set should include all the changes done to resolve that particular issue. A change set should also be compilable upon commit and not rely on the next one to be able to work.

2) Before you commit your change set you should diff every file against the repository version and see that you only commit things that were intended to commit. It's pretty easy to commit code you commented out, temporary variable names or debugging code if you don't review your own commits. If your commit is in central parts of the application or very large it is good to have another team member sit next to you when reviewing the changes.

3) When you've limited your change set to include only one issue and reviewed all the changes done, you should write a short description of the content in your change set. This is written as the "commit comment" and will be visible when you look at the log for your repository. Since your change set only should deal with one issue it is easy to write a brief description of what you've done. It's also good to include an issue id if you have an issue tracking system.

I guess many readers might think that this takes a lot of time - but think of all the time you save due to the higher quality instead! I've done this with all commits for many years now and it is very uncommon that I add bad code to the repository. It's not at all uncommon that I notice bad code while doing my personal code review upon commit tho!

My biggest problem when converting to this more professional approach was to limit my changes to only one issue. Yet today there are times when I can't commit only one change as I've fixed two (or more) issues in parallell without commiting the first one - and when they touch the same file it isn't possible to have only one change in the change set. If one of the fixes are small I usually solve this by reverting the changes for that issue temporarily while commiting the first change and then redo the changes for the next commit - but once in a while I need to write a commit comment with the dreaded word "... and ...".

I love deleting code

2010-11-11T13:25:00.003+01:00

Yesterday I read a tweet saying "the next best thing after writing code is deletig code". My response was fast, saying "personally i like deleting code more. ;)". Let me elaborate on that!

Deleting code means that you either:

found a better way to do something
found unneeded abstractions
found unneeded functionality

So, deleting code (at least when it's done on purpose ;)) is always done because you don't need it. Taking away code that you don't need is great, because the less code you have, the less can go wrong, and after the code is removed there is less to test and less to maintain.

Therefore, always strive to have as little code as possible doing the job. As they say in Extreme Programming, "Pay as you go: Build just enough to meet today's requirements".

You could get sad when removing code because it means that you or someone else have done something that could be considered a waste. Well, sometimes it was a waste but it won't be less a waste because you keep it. Most times the code you're about to delete served a purpose though, leading you to find the better solution.

So summing it up; Don't be sad about deleting code, love it like I do!

Odd way to fill your mailbox

2010-10-07T22:07:00.003+02:00

On a site of mine I get an email every time an invalid url is requested. The purpose of that is mainly to find broken links. Sometimes a misconfigured crawler may spam me with a hundred mails, but they're pretty easy to delete and it doesn't happen frequently.

Tonight however, from 21.08 to 21.53, I received more than 3.500 such, all from the same ip. It would probably have been more if i didn't block that ip at 21.53, as I luckily was at the computer. The requested url:s were all directories and pages that exist on the site, but combined in odd ways, primarily stacking directories in long chains that doesn't exist.

As I don't have access to the firewalls of my hosting company I had to figure out a way to block the weirdo myself. My solution was to just terminate requests from that host in my asp.net page like this (but the real ip instead of the x:es):


  protected void Page_Load(object sender, EventArgs e)
  {
      if (Request.UserHostAddress == "x.x.x.x") {
          Response.End();
          return;
      }
      ...
  }

The Anti-IF Campaign

2010-03-17T13:14:00.004+01:00

When I first found Francesco Cirillo's Anti-IF Campaign I signed up almost instantly. Over the years I've learned that the if-statement is best used sparsely. Neither the campaign, nor I, strives to eliminate all if-statements but rather wants you to think twice before using them. Unfortunately I don't think the web site does a good job explaining why if is bad, so I'm going to make a try on my own.

Every if-statement creates another path through the code. That opens up for a lot of additional cases to test and hence harder to test. Harder to test means there will likely be more bugs. For example you need to add at least one Unit test for each if you add to your code. A nice technique for testing your code coverage is to comment out either the if or the content and see if any test fails. If it doesn’t the code is either unnecessary or not covered by your tests.

That being said code is a lot about different paths, and must be, so you can’t take away all if-statements. But consider your options!

There’s the redundant if:

if (flag) {
 flag = false;
}

Just replace that with

flag = false;

There’s the horrible null check:

if (filter != null) {
 filteredList = filter.Filter(list);
}
else {
 filteredList = list;
}

Create a null object filter and always filter, like this.

If don’t have a interface for your filter already, create one.

interface IFilter {
 Array Filter(Array list);
}

Make a implementation of that interface that just returns the argument.

class NullFilter : IFilter {
 Array Filter(Array list) {
   return list;
 }
}

Where you used to decide not to set the filter field, create a NullFilter.

IFilter filter = new NullFilter();

Now remove the if and the else and just filter.

filteredList = filter.Filter(list);

Now lets round up with the case that Francesco lists on the Campaign site where you refactor your code to use strategy objects.

The Simplest Anti-IF Code (Anti-IF Campaign)

There's a lot of other cases, but I can't go through them all. I hope that you've learned that if should be avoided and that you will try to do so in the future.

I'll end this article with a Twitter quote from @garybernhardt.

Let's rename the "if" construct to "ponder" and impose a one-second busy wait per use. That'll teach those branchers! ;)

SRP subsumes ISP

2009-11-14T19:20:00.004+01:00

Yesterday I listened to a friend who held a presentation about the SOLID principles. You know, the five principles in object oriented design that Uncle Bob put together.

Single Responsibility Principle
Open/Closed Principle
Liskov Substitution Principle
Interface Segregation Principle
Dependency Inversion Principle

If you're not familiar with them you should read up right now. Here's a good place to start.

Anyway, when my friend reached the I of SOLID it struck me that ISP is only interesting if you've violated SRP to start with. I've always thought of it as a refactoring pattern, but all the definition says is that "make fine grained interfaces that are client specific". Adhering to the SRP it would be difficult to make interfaces any other way... I decided to confront Bob with this, and he seems to agree:

I asked:
would you agree that ISP is self-fulfilled if SRP is used? ie, ISP only has a value on its own when working with bad design?

And he replied:
Strictly speaking, SRP subsumes ISP. The problem is you can't always be as strict about SRP as you'd like.

So, ISP is mainly an option if you fail to design by the most important principle. But as Bob states, it's not at all uncommon that you do that so there's still value in the principle.

Also, I think ISP is a great principle when dealing with legacy code. If you have a huge class it may be both difficult and very time consuming to refactor it to meet SRP. Here I use ISP as a lightweight SRP, creating the interfaces that I'd like the objects to be in a good SRP design and then have the same class implement them all. It takes virtually no time, and then I can start writing good code from that point on. It also helps me if I want to refactor out one of the many parts this legacy class may contain.

If I'd end up with some kind of conslusion it would be that you shouldn't think much about ISP when designing new software, but it's a great principle for refactoring!

Learning XP

2009-10-15T17:55:00.003+02:00

I'm currently having a great time at work, developing a web application from scratch using Extreme Programming. You can follow our stumbling steps in a new blog, XP Creuna.

String versus string in C#

2009-01-19T22:27:00.006+01:00

A month ago I had a developer that I'm currently in the same project with questioning me about my use of string versus my use of String. She found it confusing that I didn't just use one of them consistently. I told her my idea, which is that I'm using String when accessing static members and string otherwise, but she still found it confusing.

I then decided to consult the IDesign C# Coding Standard, and looking there I found some inconsistency. I decided to ask them about it, and they seem to share my belief. See my mail and answer below.

From: Juval Lowy
Sent: den 19 december 2008 15:51
To: Tommy Bryntse
Subject: RE: Contacting IDesign

You should use String for the static members.

________________________________________
From: Tommy Bryntse
Sent: Friday, December 19, 2008 6:27 AM
To: sales@idesign.net
Subject: Contacting IDesign

Hello IDesign!

I noticed an inconsistency in your C# Coding Standard.

In section 1.10 it says

Always use … string NOT String.

While in section 2.52 it says

Use String.Empty instead of "":

Myself I’ve always used String for accessing static members in string, and string 
otherwise, just as your example. Section 1.10 was about to make me change that to 
always use string, but after seeing section 2.52 I’m confused. What should I do?

Kind regards,

Tommy

Deserializing non printable characters

2008-04-10T11:57:00.002+02:00

Today I got on my desk to fix a problem with our import and export functionality. In an interface open to customers one of them had entered non printable characters in a string property which caused the import to fail if the item had been exported.

My first thought was that it seemed strange that the XmlSerializer of the .NET Framework didn't automatically encode string properties to proper XML. Looking at the XML file it was correct tho. Or, proper XML format at least. To be proper XML it seems you may not use control characters other than CR, LF and TAB.

XML Spec 1.0 says:
Char ::= #x9 #xA #xD [#x20-#xD7FF] [#xE000-#xFFFD] [#x10000-#x10FFFF]

Considering this it seems like the Serialize method that generated with XML should be the one complaining, but in this case the problem is reported by the Deserialize method. It throws an InvalidOperationException saying it encountered an invalid character in a call to ScanHexEntity.

With the help of Google I did however find a simple solution to the problem. Calling Deserialize with an XmlTextReader instead of a StringReader. Since the former has a constructor that take the latter as argument it has minimum inpact on the code too.

Solution:
xmlSerializer.Deserialze(new XmlTextReader(sr));

The reason is that this XmlTextReader contructor initiates with the Normalization property set to false, while the XmlTextReader created under the hood from other calls to Deserialize is created with the property set to true.

A more sophisticated solution would be to base64 encode the string, but I'll stick with the simple solution. More than having very limited inpact and being a very quick solution it also has the benefit that old export files will work.

MSBuild Error MSB3171

2008-04-04T09:04:00.002+02:00

Converting a solution from VS2003 to VS2008 today caused me almost an hour of headache. It seems that there is some bug with changing the manifest settings in managed C++.

The solution contains a number of C# projects and one Managed C++ project. I converted its debug configuration many month ago and today I was finally going to build it in release mode!

One problem in the conversion was that most assemblies, including the C++ project, used the attribute assembly:AssemblyKeyFileAttribute. With the new studio signing is invalidated by adding the manifesto if the key file is given with this attribute, which in turn causes a binding failure when trying to load the assembly in runtime.

I remembered solving it by setting the key file in the project settings when I converted the solution in debug mode earlier. Obviously I had failed to make the change in the release configuration at that point but it should be a quick fix.

At the first glance, though, I failed to find the box where it should be entered in the C++ project settings and thought I might have removed the manifesto instead. I tried setting "Embed manifesto" to "No" in Manifest Tool\Input and Output and kept the key file as an attribute. This made the MSB3171 appear, so I tried to also set "Generate Manifest" to "No" in Linker\Manifest File. No better. Looking for other things to change I f0und the place to add the key file in the project settings and decided to go with the initial idea.

I reverted my changes regarding manifesto, but the MSB3171 problem remained. I then reverted my project files using the source control system, but the problem still remaind. Outch! I restarted Visual Studio, but the problem refused to go away.

Next step was to ask Google. I found someone that had a similar experience, but I found no answer. Others trying to reproduce his problem failed and the common conclusion was that there was something wrong with his Studio.

My next move did how ever solve the problem. I closed the Studio. I reverted all files I had changed this morning, and deleted all intermediate files in the solution that had been changed today. That is, all bin and obj directories, .ncb files, .suo files and all other files that the Studio generates. Now when I started the Studio again I got the same error as first thing in the morning, I then added the key file on the correct place in the settings for the Managed C++ projects Release configuration and now it all works!

Never Skip Curly Brackets

2008-03-14T09:05:00.001+01:00

In the book that I'm currently reading the author continuously skips curly brackets where they are not needed. That is, in if, for, while etc. statements controlling only one line of code. I would have thought that all experienced software developers always used the curly brackets.

I can't come up with any reason not to use them, except maybe "less to type" or "less lines of code". Both reasons equally lame.

How much of your programming hours are spent typing? And whats the purpose to strive for less lines of code? The purpose must be readability, and I would not agree that it is increased by not using curly brackets. Rather the opposite. If your readability problem is caused by functions having many lines it is the design that is the problem, not the brackets.

Instead, not using curly brackets is very error prone.

Consider that you want to add something to the code block of an if-statement, now you'll need to add the curly brackets anyway. And if you fail to notice this it will cause weird errors in your program since the second line will always be executed.


if( foo )
  foo = false;
  bar = true;

Or lets say that you temporarily want to remove the executed statement with a comment. Now you'll need to add the brackets, or comment the if-statement too. If you don't do any of this the if-statement will instead control the next line of code.


if( foo )
  //foo = false;

ASSERT(!foo);

As you see from these examples you may very well be fooled by the indentation of the code.

So, to all of you out there that aren't using those curly brackets unless you're forced too - start using them always!

Solving Bob's Problem

2008-03-11T21:14:00.000+01:00

Reading up on Liskov Substitution Principle last night finaly put me over the top to start blogging about software development. I had a copy of Robert C. Martin's article published on the Object Mentor webpage (direct link) where he suggest that deriving Square from Rectangle violated LSP.

Here is his code example:


class Rectangle {
public:
  virtual void SetWidth(double w) {itsWidth=w;}
  virtual void SetHeight(double h) {itsHeight=h;}
  double GetHeight() const {return itsHeight;}
  double GetWidth() const {return itsWidth;}
private:
  double itsWidth;
  double itsHeight;
};

Sparing you the declaration of the Square class, which inherits Rectangle, here's how it differs. It overrides SetWidth and SetHeight, calling them both in the base class.


void Square::SetWidth(double w) {
  Rectangle::SetWidth(w);
  Rectangle::SetHeight(w);
}

void Square::SetHeight(double h) {
  Rectangle::SetHeight(h);
  Rectangle::SetWidth(h);
}

This will violated LSP since a client may call SetHeight with one number and SetWidth with another and presume that GetHeight will return what ever number SetHeight was called with - which will of couse not be the case.

He leaves us no solution in his article, so we're left to think that a Square never should inherit a Rectangle. I myself should in this simple case make the objects immutable. Aside from all other benefits it will also make the objects pass the LSP test.


class Rectangle
{
public:
 Rectangle(double w, double h) {itsWidth=w; itsHeight=h;}
 double GetHeight() const {return itsHeight;}
 double GetWidth() const {return itsWidth;}
private:
 double itsWidth;
 double itsHeight;
};

class Square : public Rectangle
{
public:
 Square(double side) : Rectangle(side, side) {}
};