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

Question

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?

Anas Karkoukli · Accepted Answer · 2011-10-25 14:15:42Z

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!

Your first code block is convoluted and hard to skim, thus bad from a maintainability point of view. This ruins your answer, IMO. I would suggest to keep the original if null assign; afterwards return approach. — ANeves
– ANeves, Commented Oct 25, 2011 at 15:07
The succinct style I use is orthogonal to the answer, I do not believe it takes away from the validity of the answer. I had similar reaction when I first realized how versatile and multi-paradigm C# can be, after all there's a lot of personal/individual style involved in coding. — Anas Karkoukli
– Anas Karkoukli, Commented Oct 25, 2011 at 18:15
This being codereview style is not orthogonal to answers :) , and the question also asks whether the code is maintainable. But other than that, I still think your answer is very good. — ANeves
– ANeves, Commented Oct 26, 2011 at 9:38

ANeves · Accepted Answer · 2011-10-25 11:32:23Z

Looks perfect to me.

The only comment I have is that, assuming an empty Profile makes sense, the String.Empty default on the properties could be set by the constructor - rather than you setting them explicitly.

Jesse C. Slicer · Accepted Answer · 2011-10-25 16:02:17Z

Do you have .NET 4 at your disposal? If so, you can use the Lazy<T> as such:

 private readonly Lazy<Profile> individualProfile = new Lazy<Profile>(() => { var newProfile = ProfileFacade.Instance.GetMainProfileByType( PortalContext.CurrentUserId, ProfileType.IrnicIndividual); return newProfile ?? new Profile { FirstName = string.Empty, Family = string.Empty, Details = new IrnicIndividual { NationalCode = string.Empty } }; }); [DebuggerBrowsable(DebuggerBrowsableState.Never)] public Profile IndividualProfile { get { return individualProfile.Value; } }

Stack Exchange Network

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

3 Answers 3

You must log in to answer this question.

Hot Network Questions

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

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions