Excuses

I’ve set up the domain and blog more than half a year ago. I wanted to write about great new technologies and all this amazing stuff people are using them for. I wanted to learn and share my knowledge with others.

But in January I moved to Ireland. New country, new job, new language, new everything. It turned out that between work, learning and settling in a new place, I simply didn’t have enought time to write. Or at least it was good enough excuse for me.

So why writing now?

I’ve recently moved to another SharePoint project. It uses Office PnP (not my decision). How Office PnP makes it impossible to write performant code is completely different story and I’ll leave it for now.

This post was caused by one particular property and one tiny extension method in Office PnP. I seriously wanted to start with something more positive, but I just cant fight it.

For some Boolean? is just too much to handle

I started writing some basic code in the new project and this is what I see: ServerObjectIsNull

There’s a property on ClientObject:

public Boolean? ServerObjectIsNull { get; }

It has three possible values:

  • null when object wasn’t requested from the server yet,
  • true when object was requested from the server and exists,
  • false when object was requested from the server and doesn’t exist.

Each of them meaningfull and each of them usefull. But then in Office PnP we have an extension method:

public static Boolean ServerObjectIsNull<T>(this T clientObject) where T : ClientObject

It immediately got my undivided attention. Why did someone create this method if there’s already a property? What kind of magic happens inside that would make it usefull? To the repo!

The method’s implementation:

public static bool ServerObjectIsNull<T>(this T clientObject) where T : ClientObject 
{
    return (!(clientObject.ServerObjectIsNull != null &&  
        clientObject.ServerObjectIsNull.HasValue &&  
        !clientObject.ServerObjectIsNull.Value)); 
}

Just how bad can one method be?

Well, let’s see:

  • The only purpose of this method is to convert Boolean? into Boolean.
  • Important information behind null value is lost in the process.
  • Method does not give you reliable result. Possible values are:
    • true when object may be null (requested and is null, or uncertain because not requested).
    • false when object is not null.
  • That makes the name wrong. It should be: ServerObjectIsNullOrNotRequested.
  • Check this Pull request. It was actually implemented wrong! Isn’t anyone reviewing the code before accepting PRs?
  • Why is this method generic? There is absolutely no reason for that. It could just be extension method of ClientObject. Inheritance? Anyone?
  • There is no difference between clientObject.ServerObjectIsNull != null and clientObject.ServerObjectIsNull.HasValue. Elementary C# knowledge.

And finally. Let’s consider this code:

var text = "Lorem ipsum";
var isStringNull = text == null;

Is there something wrong with it? No, it’s completely valid code. Let’s take this one then:

var text = null;
var isStrinLoremIpsum = text == "Lorem ipsum";

Still fine. No NullReferenceException thrown. Is this one any different?

Boolean? nullableBoolean = null;
var isTrue = serverObjectIsNull == true;

It isn’t. Then please, can somebody explain to me, why the method has not one, but two null checks when it could be implemented like this:

public static bool ServerObjectIsNull(this ClientObject clientObject) => 
    clientObject.ServerObjectIsNull != false;

But wait…

If I can do this: web.ServerObjectIsNull != false, then why implement it as a method?