HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Public property, backing up by a private field, with caching and null checking inside

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fieldpublicwithbackingcachingnullprivatecheckingpropertyand

Problem

I have a private field called individualProfile (camelCase) and I've created a public property called IndividualProfile (PascalCase) on it:

private Profile individualProfile;
public Profile IndividualProfile
{
    get
    {
        if (individualProfile == null)
        {
            individualProfile = ProfileFacade.Instance.GetMainProfileByType
                                (
                                    PortalContext.CurrentUserId,
                                    ProfileType.IrnicIndividual
                                );
            if (individualProfile == null)
            {
                individualProfile = new Profile()
                {
                    FirstName = string.Empty,
                    Family = string.Empty,
                    Details = new IrnicIndividual()
                    {
                        NationalCode = string.Empty
                    }
                };
            }
        }
        return individualProfile;
    }
}


I have to use this public property in almost 10 places, so I first check the private member to see if it's null and load it only once, so that I load it only once and I cache it and use it 10 times. Also, since GetMainProfileByType can return null value, I check another time to see if the private member is null or not. If it's null, I simply set it to an empty object, so that I won't bother checking null values when I bind this object to my view.

Is this code efficient and maintainable? Is there anything I've overlooked?

Solution

Good thinking about the caching and efficiency. I have two minor suggestions

1 - Enforce SRP (single responsibility principal), which basically states (according to Robert C. Martin) that in the world of "Clean Code" classes must have one reason to change for better maintainability, which is one of the concerns you had in your question. So to apply this to your case, the class that contains the Profile property need not be changed if you decide to change the signature of the ProfileFacade.Instance.GetMainProfileByType method, or maybe choose another method to populate the Profile. So maybe refactoring that code out of the class would be a good idea. Here is what I mean by that:

private Profile _individualProfile;
public Profile IndividualProfile
{
    get
    {
        if (_individualProfile != null) return _individualProfile;

        return (_individualProfile = ProfileFacade.Instance.GetIrnicProfile() ?? Profile.NullProfile);
    }
}


As you can see in the code snippet I used the short hand null check using the ?? null-coalescing operator.

2 - NULL object pattern. Basically define the neutral state (or NULL state) of your object inside the object itself, benefiting from not changing the calling code if ou decide to redefine what the NULL means for your object. In my code snippet I added Profile.NullProfile which is defined as such:

public class Profile
{
    public string FirstName { get; set; }
    // Other properties here
    public static Profile NullProfile
    {
        get
        {
            return new Profile
                        {
                            FirstName = String.Empty,
                            // etc...
                        };
        }
    }
}


Other than that... Good work!

Code Snippets

private Profile _individualProfile;
public Profile IndividualProfile
{
    get
    {
        if (_individualProfile != null) return _individualProfile;

        return (_individualProfile = ProfileFacade.Instance.GetIrnicProfile() ?? Profile.NullProfile);
    }
}
public class Profile
{
    public string FirstName { get; set; }
    // Other properties here
    public static Profile NullProfile
    {
        get
        {
            return new Profile
                        {
                            FirstName = String.Empty,
                            // etc...
                        };
        }
    }
}

Context

StackExchange Code Review Q#5565, answer score: 6

Revisions (0)

No revisions yet.