TL;DR
You are only responsible to null-check your encapsulated logic (= hidden from an external caller). All other information is (by elimination) supplied by the external caller and therefore not your responsibility to null-check (it is the external caller's responsibility).
public class ChildOfTrue : BaseClass
{
public ChildOfTrue() => this.Validity = true;
}
public class ChildOfFalse : BaseClass
{
public ChildOfFalse() => this.Validity = false;
}
This is not good practice. Derived classes are created when they introduce structural differences from their base class. What you're doing is simply changing a value. This is no different from doing things like:
public class John : Person
{
public John() => this.Name = "John";
}
public class Five : MyNumber
{
public Five() : base(5) {}
}
However, it's possible that your example class is oversimplified for the purpose of example. Regardless, I do question your intention to override a boolean property's value based on some derived classes.
public bool IsValid()
{
return this.Validity;
}
I don't see the point of this method. It's nothing more than a property accessor.
Why not add the property to the interface? The method has no added value anyway.
public IValid IValidity
{
bool Validity { get; }
}
Attempt #2 - introduce extension:
If it's to avoid null
issues, I can tell you that it's not going to help. Extension methods are matched on variable type (BaseClass
). Even if your BaseClass
variable is null, you will still be able to compile and run this.
Yes, you can technically wrap a null check in this method. But your solution is probably going to have more than one class. All classes are nullable, so the problem exists for all classes.
Your current approach would lead you to have to make a similar null-wrapping check for every method that exists in every class that exists. Or, at the very least, for every method or property that is the implementation of an interface.
But since this applies to every class, even if it doesn't have an interface, you should really be doing this on object
level. Oh wait, but then you can't actually wrap your class-specific methods anymore!
Trying to create a wrapper method for every public method/property for every class/interface is an exercise in futility, will not lead to cleaner code, and is going to be maintenance hell when you get to a sizeable codebase.
Attempt #3 - introduce helper class instead of extension:
Extension methods are really just helper methods with a nicer syntax. There is no technical difference between #2 and #3.
Summarization
What you're trying to do here is go around the intention of the language. Consider this simple variable:
BaseClass myObj = new BaseClass();
It's important here to distnguish the inside (BaseClass
methods and properties, what we call "encapsulated logic") from the outside (the client code that declared this variable). This distinction is important, because null
exclusively lives on the outside.
An object cannot know if it's null. By definition, if this object exists, it's not null.
This is like trying to think "what would I be thinking if I didn't exist?". You wouldn't be. That's the entire point of defining "not existing".
The absence of something can only be detected by an outside party that observes the object (or lack thereof). An object cannot detect its own absence, since being able to check for an absence inherently requires an object to exist in the first place. The premise defeats the intention.
Attempt #1 is correct. However, there are syntactical improvement to be made that create cleaner code:
foreach (var testSubject in testSubjects)
{
if (testSubject?.IsValid() ?? false)
{
/* Perform tests. */
}
}
Sidenote: you can also do testSubject?.IsValid() == true
instead. However, I find that == true
SEEMS more redundant than it is. ?? false
clearly communicates to the developer who reads the code that it's written to handle cases of null
.
or, as an alternative approach:
foreach (var testSubject in testSubjects.Where(ts => ts != null))
{
if (testSubject.IsValid())
{
/* Perform tests. */
}
}
or, my preferred option: not checking for null (in certain situations).
In your example code, you willfully introduced null
. In my opinion, the code should blow up in your face: you introduced nonsensical data.
Similarly, if this is a method which receives a list of objects from an external caller, that external caller gave invalid input and therefore needs to be made aware of that.
To summarize the intention of choosing to ignore null checks:
When the giver of input and the receiver of null reference exceptions is the same party, the exception can be considered good feedback. More casually, I call this "shit goes in, shit comes out, no big deal".
For example:
public string GetNameOfPerson(Person p) => return p.Name;
You supplied null
to this method, which is obviously going to ensure that this method can't do what you ask it to do. You deserve the exception.
However, if the giver of input is different from the receiver of the NRE, that does mean you need to handle the null. For example:
public string GetNameOfPerson(int personId)
{
var person = myDatabase.GetPerson(personId);
return person.Name; //possible null reference
}
The external caller supplied an ID. He cannot know if the underlying database actually has a person or not. Therefore, he does not deserve to receive a null reference exception, as he is not the one who introduced the null.
In other words, you are only responsible to null-check your encapsulated logic (= hidden from an external caller). All other information is (by elimination) supplied by the external caller and therefore not your responsibility to null-check.
There are two solution here. Either you give a more sensical exception:
public string GetNameOfPerson(int personId)
{
var person = myDatabase.GetPerson(personId);
if(person == null) throw new Exception("Person could not be found.");
return person.Name; //possible null reference
}
Or you return a default value:
public string GetNameOfPerson(int personId)
{
var person = myDatabase.GetPerson(personId);
return person?.Name ?? "John Doe";
}
The correct solution is contextual. Generally speaking, you'd want to avoid throwing exceptions, but there are valid use cases for throwing exceptions.
abstract
method in your base class that forces every child to implementIsValid
?