Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've inherited a class which has a bunch of properties defined as:

public int ID
{
    get { return m_nOID; }
    set { m_nOID = value; }
}

public int ConxnDetail
{
    get { LoadDetails(); return m_nConxnDetail; }
    set { m_nConxnDetail = value; }
}

public bool ConxnConstraints
{
    get { LoadDetails(); return m_bConxnConstraints; }
    set { m_bConxnConstraints = value; }
}

public bool IsShowCables
{
    get { LoadDetails(); return m_bIsShowCables; }
    set { m_bIsShowCables = value; }
}

public int DefaultQueue
{
    get { LoadDetails(); return m_nDefaultQueue; }
    set { m_nDefaultQueue = value; }
}

private User LoadDetails()
{
    if (!IsLoaded && !IsNew)
    {
        Logger.InfoFormat("Loading user details. UserID: [{0}]", m_nOID);
        User user = RemoteActivator.Create<RemoteUser>().GetUserById(m_nOID);
        if (user != null)
        {
            CopyDetails(user);
            MarkLoaded();
        }
        else
            ClearDetails();
    }
    return this;
}

I would absolutely love to clean up the conventions here a bit and use only automatic getters/setters. Is this possible to do? If not, any other suggestions on reducing the boilerplate needed?

share|improve this question
    
Is there a reason the data isn't loaded when the object is constructed? –  Winston Ewert Jan 16 '13 at 18:44
    
Retrieving the ID property does not incur loading of all details. I assume that this was an optimization made at some point to lazy-load everything except the ID. –  Sean Anderson Jan 16 '13 at 18:55
    
@SeanAnderson What is your intention? To keep that optimization in or to remove it? –  svick Jan 16 '13 at 19:12
    
My intention is to not alter the function of the code, but to improve its readability. At the very least, is there a way to listen for properties being retrieved and execute LoadDetails only if the loading property isn't ID? This would allow me to replace all of this with automatic properties. –  Sean Anderson Jan 16 '13 at 19:14
add comment

1 Answer 1

up vote 3 down vote accepted

There is the INotifyPropertyChanged event handler, but I'm not sure why you would need that in this case? Instead why not just load the details when the ID changes as that seems to be the only value that will change between LoadDetails calls.

Perhaps something like:

public int ID
{
    get { return m_nOID; }
    set {

       if(m_NOID != value) {
          InitialiseUserDetails(m_nOID);
       }

       m_nOID = value; 
    }
}

If this was ok, you could remove the private backing fields as I believe you wish to do.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.