Update:
I actually thought of a method this morning that would work with your specific case and be a lot easier to maintain. I'll leave the original two points here, but I'm going to recommend the final option instead, so skip to the "better method" section.
Create a ThrowIfReadOnly method, which does what it says on the tin. This is slightly less repetitive and avoids the nesting.
Use an interface. Have an IPage that implements the functionality you want, have every public method return an IPage, then have two implementations, an EditablePage and a ReadOnlyPage. The ReadOnlyPage just throws an exception whenever someone tries to modify it. Also put an IsReadOnly property (or State property) on the IPage interface so consumers can actually check the status without having to catch an exception.
Option (2) is more or less how IList and ReadOnlyCollection<T> work together. It saves you the trouble of having to do a check at the beginning of every method (thus eliminating the risk of forgetting to validate), but requires you to maintain two classes.
-- Better Method --
A proper technical spec would help a lot to clarify this problem. What we really have here is:
- A series of arbitrary "write" actions;
- Each action has the same outcome, dependent on the state:
- Either the action is taken (unpublished/reworking), or fails/no-ops (read-only).
What really needs to be abstracted is not so much the action itself, but the execution of said action. Therefore, a little bit of functional goodness will help us here:
public enum PublishingState
{
Unpublished,
Published,
Reworking
}
public delegate void Action();
public class PublishingStateMachine
{
public PublishingState State { get; set; }
public PublishingStateMachine(PublishingState initialState)
{
State = initialState;
}
public void Write(Action action)
{
switch (State)
{
case PublishingState.Unpublished:
case PublishingState.Reworking:
action();
break;
default:
throw new InvalidOperationException("The operation is invalid " +
"because the object is in a read-only state.");
}
}
}
Now it becomes almost trivial to write the classes themselves:
public class Page
{
private PublishingStateMachine sm = new
PublishingStateMachine(PublishingState.Unpublished);
private string title;
private string category;
// Snip other methods/properties
// ...
public string Title
{
get { return title; }
set { sm.Write(() => title = value; }
}
public string Category
{
get { return category; }
set { sm.Write(() => category = value; }
}
public PublishingState State
{
get { return sm.State; }
set { sm.State = value; }
}
}
Not only does this more-or-less implement the State pattern, but you don't need to maintain separate classes or even separate code paths for the different states. If you want to, for example, turn the InvalidOperationException into a no-op, just remove the throw statement from the Write method. Or, if you want to add an additional state, like Reviewing or something like that, you just need to add one case line.
This won't handle state transitions for you or any really complex actions that do different things depending on the state (other than just "succeed" or "fail"), but it doesn't sound like you need that. So this gives you a drop-in state implementation that requires almost no extra code to use.
Of course, there's still the option of dependency injection/AOP, but there's obviously a lot of overhead associated with that approach, and I probably wouldn't use it for something so simple.
