1

So in a way I need a setter or a method that is only available to one (or a few more) classes. As far as I know this is common in some other languages as friend classes. I tried to simplify the example below. It's not the real case, and it's a bit more complicated. But I have to make sure that two properties in two different classes keep in sync. I.e. I'm using Entity Framework, and these properties are being synced to the Database.

public class UserGroup
{
    List<GroupMember> _Members;
    int _AcceptedMemberCount; // this is being synced to the database on save for performance reasons.

    public void Join(User user)
    {
        var member = GetOrCreateMember( user );
        member.State = MemberState.Accepted;
        UpdateAcceptedMemberCount();
    }

    public void Ban(User user)
    {
        var member = GetOrCreateMember( user );
        member.JoinState = MemberState.Banned;
        UpdateAcceptedMemberCount();
    }

    private GroupMember GetOrCreateMember(User user)
    {
       // Creates a GroupMember from User,
       // and add it to _Members if it doesn't exist.
    }

    private void UpdateAcceptedMemberCount()
    {
         _AcceptedMemberCount = _Members.Where(m => m.State == MemberState.Accepted).Count();
    }

}

public class GroupMember
{
    public MemberState State { get; internal set; }
}

Alternatives that I know

Using internal

disadvantage is that the property is still available to all the other classes in the same assembly. I.e. I have an assembly with my business models and entities. Making a separate assembly would complicate the code, especially when you're using Entity Framework, and need to start splitting the entities over multiple assemblies.

Adding a SetState(UserGroup userGroup) to GroupMember

... that does some extra manual checking if the UserGroup has done it's work. Like checking if the User has been added as a GroupMember to _Members. Feels really ugly, and more code to maintain that could get out of sync and break down in the future.

Using nested classes (See: https://stackoverflow.com/a/10507130/647845)

  • Does not really work well with Entity Framework.
  • Makes the code more verbose (always adding the parent class as a prefix when working with a GroupMember).
  • GroupMember is an important standalone class in this application. Hiding it behind a parent class for just this reason feels bad.
  • Has limited use, because every class can only have one hierarchy of outer classes.

Document it in the method summary, and ignore it

And just hope that everyone reads it, and never starts fiddling with the State of a GroupMember manually. The disadvantages of that are probably clear :)

Question

I can't imagine this is a really weird example, and lots of applications must have the same problems. What is the best practice here? How is this commonly solved?

Thanks in advance,

Community
  • 1
  • 1
Dirk Boer
  • 8,522
  • 13
  • 63
  • 111
  • Maybe I just missed it, but I find myself unable to conclude which properties you want to keep synced. – MicroVirus Aug 31 '14 at 10:37
  • Sorry, maybe I screwed up a bit with simplifying my real world example. I've updated the example. – Dirk Boer Aug 31 '14 at 10:39
  • 1
    Verifying that the correct state has been set is something for Unit-testing anyway. Controlling who can set it shouldn't be so overall important. Combine `internal` with "Document it". – H H Aug 31 '14 at 10:43
  • Hi @HenkHolterman, isn't this actually something that is explicitly not really working well with unit testing? Both methods work as expected. If someone adds his own `MyOwnBan()` method on the GroupMember, he might not notice that it is essential that AcceptedMemberCount is updated, and the Unit-tests that are in place would not fail. – Dirk Boer Aug 31 '14 at 10:50

1 Answers1

0

If it's acceptable for other classes to modify GroupMember.State, then you could use events to let the UserGroup monitor whenever a state has changed: when State changes, it raises an event and the UserGroup class listens to it.

If only the UserGroup should be allowed to set state, or if at least any state change should pass by user group, then one thing you could do is move the actual storage of the State property away from GroupMember and into UserGroup:

public class UserGroup
{
    List<MemberState> _MemberStates;
    ...
}

This would require each GroupMember to store the UserGroup it belongs to, so this will only work if each group member is part of only one usergroup. If this is the case then GroupMember will look something like:

public class GroupMember
{
    private UserGroup _Parent;

    public GroupMember(UserGroup parent)
    {
        _Parent = parent;
    }

    public MemberState State { get {return _Parent.GetState(this);} }
    // Where GetState is a function that returns the state from _MemberState

    ...
}

Of course, this does create a strong coupling between the two (together they work to provide two sides of the same public interface) and only works if each GroupMember always has exactly one UserGroup.

Now, if each group member can be part of multiple groups, then the above won't work, but guessing from your example this isn't the case here (because friend wouldn't solve this), so I'll not go into possible solutions.

After thinking about it some, it really comes down to your exact requirements and design choices. It's impossible to give a one-size-fits-all answer, but maybe I managed to inspire you to find a good choice for your project.

MicroVirus
  • 5,324
  • 2
  • 28
  • 53