Fix your code, don’t disable static analysis

Maybe it is my OCD, maybe it is that I would like to think I try to always write clean code, maybe it is something else entirely. But I always cringe when I see people turn off or disable static analysis in their code.

The reason I cringe is because I have to assume that the authors of the static analysis tools (be it ReSharper or Visual Studio or another product) are more knowledgeable in these areas than I am and they better understand why it is bad to so something.

Today I came across this ‘// ReSharper disable once PossibleMultipleEnumeration’ inside a method and not just once, but twice.

Take a look at the code below.

private ReturnValuesForDateJson[] GetReturnValues(IEnumerable<ReferenceNumberAndReturnTypeRecordModel> recordModels)
  // ReSharper disable once PossibleMultipleEnumeration
  var dates = recordModels.First()
  	.ReturnRecords.Select(x => x.ReturnDate).Distinct();

  return dates.Select(aDate => new ReturnValuesForDateJson
      ReturnDate = new MonthJson(aDate.Year,aDate.Month),
      // ReSharper disable once PossibleMultipleEnumeration
      ReturnValues = GetReturnValueForSeriesIdentifier(aDate, recordModels)


Notice how the ReSharper warning for PossibleMultipleEnumerations has been disabled 2 times, this is because the method argument is an IEnumerable. If we change this IEnumerable to either IList or ICollection and the errors go away or we can leave the argument as IEnumerable and get the list by calling .ToList() on the Enumerable.

Now why is it an issue iterating over an enumerable multiple times? Because Enumerable collections are evaluated each time you go over them the underlying results could possibly change. Imagine that you pass in a LINQ statement into the method. The nature of LINQ would allow the results to be different each time, thus possibly causing bugs or errors.

Working code, no more static analysis errors

private ReturnValuesForDateJson[] GetReturnValues(ICollection<ReferenceNumberAndReturnTypeRecordModel> recordModels)
  var dates = recordModels.First()
  	.ReturnRecords.Select(x => x.ReturnDate).Distinct();

  return dates.Select(aDate => new ReturnValuesForDateJson
      ReturnDate = new MonthJson(aDate.Year,aDate.Month),

      ReturnValues = GetReturnValueForSeriesIdentifier(aDate, recordModels)


Remember, if the tool is telling you that your code is less than optimal give it a look and try to fix it. Now, there may be legitimate reasons to ignore the warnings, that is cool. But when this is the case do your friends a favor and add a comment regarding the intent so future developers understand the line of thinking.

Till next time,

Posted in Clean Code | Tagged | Leave a comment

Merge Headache — Don’t Re-Purposes a class, create a new one

Merging code does not have to be the frustrating process that many people experience, if done right. I have learned during my career that if I pull from my master branch daily, if not more often, my merges are almost always pain free. Now I am not saying that merging is always pain free. There will be times where significant or simultaneous changes to a file introduce pain. There will also be times when a codebase is undergoing structural changes that you will experience issues, but honestly if the changes to the code are standard I attest that merging code should not be too painful.

However, there are things developers can do which directly introduce pain and frustration into the merge process. One of these things is re-purposing a file with almost an entirely new code base. When I say re-purposing, I mean NOT changing the name, but changing 80% of the logic inside the file.

Imagine you have a file called Baz.cs and inside of this file you have a class named Baz. Now image you made changes to the Baz class, inside the Baz.cs file. Now, at the very same time you are making changes to Baz.cs someone else (in another branch mind you) is also making changes to Baz.cs. However, their changes not only change the contents of the Baz class, they rename the Baz class to Bar. While renaming Baz to Bar they also introduce signficnat changes to the body of the class.

The issue here is that when you attempt to do a merge or rebase your SCM system is going to flag almost every line with a conflict, but not all lines (depending on the changes of course). This will leave the person doing the merge to have to make a decision on what to do. Do you take the changes as is, are your changes still needed, do you need to create a new class/file so that you can have both set of changes? What should you do? How do you complete the merge without blowing the prior changes out of the water?

To avoid this type of merge headache what should you do? In my opinion, if you are going to change the intent of a class or the intent of a file you should create a NEW file and delete the old one. If this process had been followed in this scenario the Baz.cs file would have been deleted and the Bar.cs file would have taken its place. This would have allowed me to not deal w/ the merge issues if I knew the file was no longer needed or possibly undo the delete if I know my original changes are still needed.

Keeping the merge process pain free is not too hard, but it does require a bit of fore thought and planning.

Till next time,

Posted in Git, Uncategorized | Tagged | 3 Comments

Typescript Support in Atom Editor for Windows

Recently I was trying to get TypeScript support working inside the Atom editor on windows.

In my attempt to get things working I went to the Atom site and found the TypeScript package. Per the documentation I did ‘apm install typescript’. After about 15 seconds it appeared that I was good to go. Sadly this was not the case. When I opened Atom (by typing in atom on the cmd prompt) I would receive this error.

Because I like to follow directions I restarted Atom (again via the CMD prompt). Sadly I received the same error again… WTF.

Well a quick google search for ‘These are now installed. Best you restart atom just this once.’ yielded one result. However, when I clicked on the link I was taken to the github 404 page, seems that link is dead. What to do now? Lucky for me there was a cached version of the page I could look at (thank you google).

Looking through the source file I was able to find the block of code which was throwing this message (seen below)

It appears that both linter and autocomplete-plus are required in order for TypeScript support to work. I assumed these would have been installed by default, but guess not.

I thought I would simply try to install these Atom packages in hopes the error would go away. To accomplish this I ran the following 2 commands

  • apm install linter
  • apm install autocomplete-plus

Once I had both of these packages installed I tried to reopen Atom. To my excitement the TypeScript message was no long present. To ensure my fix worked I decided to edit a .ts file and yup, my stuff recompiled down to js…

Hope this helps,

Till next time,

Posted in Uncategorized | 3 Comments

JavaScript Code Coverage using Karma-Coverage w/ Grunt

As part of our ongoing effort at my client to setup a testing environment for our JavaScript code I wanted to also setup the ability to do code coverage on our files.

To accomplish this I am going to integrate istanbul coverage reporting w/ our karma test runner via the karma-coverage plugin.

** I am going to assume you already have JS tests running w/ Karma and Grunt **

To accomplish this we first need to install the following NPM packages

  • npm install istanbul –save-dev
  • npm install karma-coverage –save-dev

Next thing we need to do is open our karma.conf.js file and make some changes to it.
1) Update the reporters configuration

reporters: ['progress', 'coverage'],

2) Add a preprocessor section to the configuration.

    preprocessors: {
        // source files, that you wanna generate coverage for
        // do not include tests or libraries
        // (these files will be instrumented by Istanbul)
        '**/js/page/**/*.js': ['coverage']

3) Setup the coverage reporter. This is the outputted format of the results.

    coverageReporter: {        
        dir: '../../../grunt/js.coverage/',
        reporters: [
                { type: 'html', subdir: 'report-html' },                
                { type: 'teamcity', subdir: '.', file: 'teamcity.txt' },

In my setup I am doing 2 things.

  • I am placing my coverage files inside my grunt working directory. This means I need to back navigate to the folder.
  • I am outputting to both HTML and teamcity format. You do not need to specify more than one format if you do not want or need to.

3) I added the karma-coverage plugin to the plugin section. When I left this out I would get an error, adding this resolved the missing plugin error.

   plugins: [

After you have made the following changes you should be able to run karma via grunt as you normally would and boom, you have code coverage for you Jasmine JavaScript files.

Till next time,

Posted in Grunt, Jasmine, Testing | 3 Comments

Forcing MVC Model State to invalid for Unit Tests

Unit testing ASP.Net MVC applications is easier than every today. But how do you force ModelState.IsValid to be false in a unit tests?

The simple thought would be to simply create an invalid object and pass that into your action method, but this will not work. Why? Because the validation is down by the MVC pipeline prior to reaching your actual method and you do not have direct access to this.

However, we can fake it by manually adding model errors, thus getting IsValid to return false.

Imagine you have a controller which does something like below

public ActionResult CustomerFeedback(CustomerFeedbackData model)
    if (!ModelState.IsValid)
        return Json(new ResponseModel<CustomerFeedbackData>());


    return Json(new ResponseModel<CustomerFeedbackData>(model));

And you wanted to create a unit test that would exercise the failing of the .IsValid check.

To accomplish this you could manually force a model error like below.

public void CustomerFeedback_When_Model_Is_Not_Valid_Will_Return_Error_State()
    var controller = GetController();  // helper method to construct an instance of the controller
    var customerFeedbackData = new CustomerFeedbackData();

    // force validation error --> this is the magic sauce
    controller.ModelState.AddModelError("FirstName", "First Name is Required");

    var result = (JsonResult) controller.CustomerFeedback(customerFeedbackData);

    var asModel = (ResponseModel<CustomerFeedbackData>) result.Data;


Long story short, you can force errors by adding them to the ModelState instance on the controller.

Till next time,

Posted in MVC | 3 Comments