views:

103

answers:

5

I have a C# project in which I use the Memento/Command pattern to implement Undo/Redo functionality. The application is a WPF application that uses StructureMap for IOC, has extensive unit tests and makes use of mocking via interfaces using RhinoMocks.

In my code I have an Operation class which represents all undoable operations, and I also have an IRepositoryWriter interface through which all writes to the database are routed.

I am wondering what is the best way to enforce the policy that only Operation and its derived classes should be able to use IRepositoryWriter.

The obvious way to achieve this would be to make IRepositoryWriter a protected, nested interface of Operation.

Advantages: only Operation and derived classes have access to IRepositoryWriter.

Disadvantages: no longer can be used with StructureMap or Unit Tested.

What are some other solutions to this? The policy doesn't need to be strictly enforced - just enough to give a hint to someone else working on the codebase would be enough.

+1  A: 

The policy doesn't need to be strictly enforced - just enough to give a hint to someone else working on the codebase would be enough

/// <summary>
///    blah blah, what this interface is for
/// </summary>
/// <remarks>
///   This interface should only be implemented by inheritors of Operation.
/// </remarks>
public interface IRepositoryWriter{}

Just stating the obvious solution for you. ;)

Jamiec
:) thanks - needless to say I'd considered that. To pick nits: it's not that the interface should only be implemented by inheritors of Operation - it's that the methods on the interface should only be *called* by inheritors of Operation.
Groky
+2  A: 

You could divide your application into multiple libraries like this:

Lib1
    UI code
Lib2
    Operation and derived classes
Lib3
    IRepositoryWriter and implementation

Only Lib2 should have a reference to Lib3. You could even use the InternalsVisibleToAttribute to ensure that internal components of Lib3 are only visible to Lib2 and make IRepositoryWriter and implementation(s) internal.

This way you can still have all the unit tests you want. Of course there is nothing that prevents another developer from creating a reference from Lib1 to Lib3 but that's a lot easier to enforce.

Ronald Wildenberg
Thanks, yes something I'd considered, and would work well. However I'm reluctant to split my code into 3 different libraries based on this. To me, libraries should ideally delineated on functionality rather than implementation details (which is what I see this as doing).
Groky
I do not quite agree. Your IRepositoryWriter (and implementation and supporting classes) contains functionality for writing to a database. I suppose you also have some IRepositoryReader or other data-reading code? That could also be part of Lib3 and that would give you a data access component, which i.m.o. is a logical candidate for a separate library. I agree that the separation of Operation and related classes into a separate library is more debatable but given your requirement I do think it makes sense.
Ronald Wildenberg
A: 

Probably overkill in this case but if you absoloutely must adhere to the DRY principle, you could write some script to alter only the .cs files that need the interface - the script could read a file that is just a list of classes that are allowed to implement the interface. That way you can nest the interface in each class that uses it, including unit tests without any duplication of knowledge. Of course the downside is that youre introducing a whole new level of complexity.

In hindsight, probably would do more harm than good, given that you can just add a comment to warn other devs of what you're doing but at least its a different perspective on the problem for you.

rmx
Wow, yes, indeed very heavyweight! And interesting idea, but I think I'd rather just go with comments if it's a choice between the two :)
Groky
+1  A: 

What about replacing IRepositoryWriter with an RepositoryWriter with a constructor that takes the Operation as a parameter?

Doobi
Something like this was my preferred solution before I asked the question ;) Though you don't need to replace IRepositoryWriter - you make RepositoryWriter the concrete implementation of that interface and make it require an Operation as a parameter to its constructor. That way it can still be mocked.
Groky
A: 

The internal keyword restricts access to a class to elements within a single assembly, i.e. an internal class isn't visible outside of the assembly - put Operation, Operation subclasses, and IRepositoryWriter into a single assembly by themselves

Mark Mullin
Make which classes internal? And how does this work with unit testing?
Groky
You would need to unit test against the Operation class hierarchy - you are asking for constraints that you too are subject to - the real issue is to what degree you want the policy enforced - technically, I agree with the responses that provide a means to advise, but eschew the absolute enforcement - my response was only to provide the mechanism for absolute enforcement
Mark Mullin