2

After reviewing a pluralsite course by Jason Roberts, I am trying to convert some of our code base to be in a more functional style.

For example, if we have:

    var request = new RetrieveAttributeRequest
    {
        EntityLogicalName = EntityTypeDictionary.First(kvp => kvp.Key.Name == entityTypeName).Value,
        LogicalName = attributeName
    };

    var response = (RetrieveAttributeResponse) _organizationService.Execute(request);

I can simply put my cursor over request, and use the ctrl+alt+N resharper shortcut and the result will be:

        var response = (RetrieveAttributeResponse) _organizationService.Execute(new RetrieveAttributeRequest
        {
            EntityLogicalName = EntityTypeDictionary.First(kvp => kvp.Key.Name == entityTypeName).Value,
            LogicalName = attributeName
        });

Another example:

We start with:

    var request = new RetrieveAttributeRequest
    {
        EntityLogicalName = EntityTypeDictionary.First(kvp => kvp.Key.Name == entityTypeName).Value,
        LogicalName = attributeName
    };

    var response = (RetrieveAttributeResponse) _organizationService.Execute(request);

    var attribute = response.AttributeMetadata;
    var type = attribute.AttributeType;
    var logicalName = attribute.LogicalName;

And after replacing everything inline:

    var logicalName = ((RetrieveAttributeResponse) _organizationService.Execute(new RetrieveAttributeRequest
    {
        EntityLogicalName = EntityTypeDictionary.First(kvp => kvp.Key.Name == entityTypeName).Value,
        LogicalName = attributeName
    })).AttributeMetadata.LogicalName;

Question

Does replacing variables with their inline equivalents increase code safety and decrease null reference pointer exceptions?

Alex Gordon
  • 57,446
  • 287
  • 670
  • 1,062
  • Why would it? Look at your last snippet -- if `Execute()` returns `null`, accessing `AttributeMetadata` will trigger a NRE. – Frédéric Hamidi Dec 14 '16 at 15:28
  • Neither is more or less prone to null reference exceptions than the other. All of the exact same references are being dereferenced - if any is null, you will get a NRE. – Ant P Dec 14 '16 at 15:28
  • The most faulty code would be something resharper didn't changed: `EntityTypeDictionary.First(kvp => kvp.Key.Name == entityTypeName).Value` when `EntityTypeDictionary` contains no elements – Jeroen van Langen Dec 14 '16 at 15:29
  • You may be getting confused by the fact that the refactored code appears to contain less dereferencing. This is true, but it's irrelevant: the first dereference that fails throws an exception and makes the rest of the code unreachable. An imperative style could have 100 dereferences while a functional style has only 1, but each would only throw a single NRE if your reference actually is `null`. So, in short: no. – Jeroen Mostert Dec 14 '16 at 15:35
  • @JeroenMostert that's a beautiful answer. Could you point me to a source that will help me wrap my brain around how to transform the code to be a more "true" functional style? What is the essence of what I need to do in order to make the code both more "functional" and allow less room for NRE? – Alex Gordon Dec 14 '16 at 15:37
  • 1
    That is a highly subjective question to which there is no good answer that would fit Stack Overflow. Personally I'd focus less on "more functional" and more on "validate your arguments" and "don't return `null`". Properly written code has no NREs because the very *first* call with a `null` argument throws `ArgumentNullException` (easy to debug), and never dereferences a `null` because the method either throws and never returns or is documented to return `null` and is checked on the client side. What's left are true bugs, and there's just no silver bullet against those. – Jeroen Mostert Dec 14 '16 at 15:41
  • 1
    See also [this proposal](https://github.com/dotnet/roslyn/issues/5032); the best defense against `null` is arguably to eradicate it from the language. But that's no easy feat. – Jeroen Mostert Dec 14 '16 at 15:44
  • I don't mean for it to be subjective. Perhaps the question should have been "how do we deal with NRE in a functional way" ? – Alex Gordon Dec 14 '16 at 15:45
  • Functional languages solve this with structured types like Haskell's `Maybe`. The language then ensures you can't miss checking for it because your patterns won't match otherwise. This just doesn't work in O-O languages -- the equivalent of `Maybe` is `Nullable`, but since there's no structured typing, the language can't enforce that you check before you do `t.Value`. O-O approaches to this include the [Null Object pattern](https://en.wikipedia.org/wiki/Null_Object_pattern), as detailed in Ross's answer. – Jeroen Mostert Dec 14 '16 at 15:53
  • @JeroenMostert thank you again, that is very insightful and clarifies a lot for me. When would it not be advisable to use the null object pattern? I don't understand why it's not used more widely? – Alex Gordon Dec 14 '16 at 15:57
  • 1
    The null object pattern isn't optimal because all it can do is give some reasonable do-nothing implementation when there's a `null`. Such an implementation doesn't always exist. Sometimes something is really supposed to be happening, and not having it happen is an error. But certainly, if you are content with not having anything happen rather than an exception, then null objects (and the [null conditional operator](https://msdn.microsoft.com/library/dn986595)) are just what you want. – Jeroen Mostert Dec 14 '16 at 16:05
  • Also, another really important reason why null objects are less used is that languages usually have no explicit support for them. Meaning that you have to write an implementation yourself. Imagine a class with 11 methods, none of them virtual, and now you have to somehow come up with an object that does nothing when each of those 11 methods is called... this requires rewriting your code in a most tedious fashion. This can be mitigated by zealously using interfaces and/or reflection and/or an IL rewriter, but those things aren't free either. – Jeroen Mostert Dec 14 '16 at 16:19
  • "by zealously using interfaces" - can you give an example? – Alex Gordon Dec 14 '16 at 16:30
  • 1
    If you have `IMyClass`, you can easily produce a `MyClassNull` that implements `IMyClass` such that every method does nothing. Your null object is then `public static readonly IMyClass Null = new MyClassNull()`. These implementations could even be generated by a template or through reflection. The obvious problem is, of course, that you need to have an interface for *everything*. Some people have that anyway for unit tests, but if you don't, it's refactoring time. (Also, you need to have a strategy for methods that return values -- what are they going to return? More null objects? Exceptions?) – Jeroen Mostert Dec 14 '16 at 16:41

2 Answers2

2

Your were not checking for null pointers to begin with. If you had code similar to:

var response = (RetrieveAttributeResponse) _organizationService.Execute(request);

if(response==null) throw ArgumentNullException();
var attribute = response.AttributeMetadata;

Then resharper would not offer the same sugestion. I think a more fluid approach would be to inline your null handling to align with your expectations. Extension methods, for example.

var response = _someObject.FluidComposition(...).ExceptionIfNUll().ExecuteSomething();

OR

var response = _someObject.FluidComposition(...).EmptyIfNUll().ExecuteSomething();

You can clearly see the intended behavior above, however, it only defines the what is expected of an operation, not the number of null pointer exceptions you will have.

Ross Bush
  • 14,648
  • 2
  • 32
  • 55
2

No. The examples you gave are functionally equivalent and the purpose of the style changes is to increase readability.

That is the same reason object initializers have been added to the language syntax as well. For example

var myClass = new MyClass()
myClass.Value1 = someValue1;
myClass.Value2 = someValue2;

vs.

var myClass = new MyClass()
{
    Value1 = someValue1,
    Value2 = someValue2
};

It is functionally equivalent, but is much easier to read quickly.

When writing code, you're writing in a language not just for the computer to read and interpret, but for other developers to read as well.

On your last example, if you wanted to do null handling, you would do something like this:

var logicalName = ((RetrieveAttributeResponse) _organizationService.Execute(new RetrieveAttributeRequest
{
    EntityLogicalName = EntityTypeDictionary.FirstOrDefault(kvp => kvp.Key.Name == entityTypeName)?.Value,
    LogicalName = attributeName
}))?.AttributeMetadata?.LogicalName;

if (logicalName == null)
    throw new Exception("Something broke.");
Brett Allen
  • 5,297
  • 5
  • 32
  • 62