13

I'm contemplating two different class designs for handling a situation where some repositories are read-only while others are read-write. (I don't foresee any need for a write-only repository.)

Class Design 1 -- provide all functionality in a base class, then expose applicable functionality publicly in sub classes

public abstract class RepositoryBase
{
    protected virtual void SelectBase() { // implementation... }
    protected virtual void InsertBase() { // implementation... }
    protected virtual void UpdateBase() { // implementation... }
    protected virtual void DeleteBase() { // implementation... }
}

public class ReadOnlyRepository : RepositoryBase
{
    public void Select() { SelectBase(); }
}

public class ReadWriteRepository : RepositoryBase
{
    public void Select() { SelectBase(); }
    public void Insert() { InsertBase(); }
    public void Update() { UpdateBase(); }
    public void Delete() { DeleteBase(); }
}

Class Design 2 - read-write class inherits from read-only class

public class ReadOnlyRepository
{
    public void Select() { // implementation... }
}

public class ReadWriteRepository : ReadOnlyRepository
{
    public void Insert() { // implementation... }
    public void Update() { // implementation... }
    public void Delete() { // implementation... }
}

Is one of these designs clearly stronger than the other? If so, which one and why?

P.S. If this sounds like a homework question, it's not, but feel free to use it as one if you want :)

devuxer
  • 41,681
  • 47
  • 180
  • 292
  • 2
    Why is it necessary to distinguish between the two kinds of repository? – John Saunders May 10 '10 at 18:17
  • @John, well, perhaps it's not, but I was thinking that I want to make sure I protect database tables that are not supposed to change from accidental edits. – devuxer May 10 '10 at 18:18
  • 2
    @DanM: what's your DBA have to say about that? Most will feel that the place to protect the database is in the database, not in your code. – John Saunders May 10 '10 at 18:22
  • @John: I am the DBA :) The database in this case is barely more than a data store with some foreign keys. There's nothing in the database to prevent inserts/updates/deletes to tables that are supposed to be read-only. – devuxer May 10 '10 at 18:28
  • @John: Protecting the database from accidental stupidity via code is not wholly unreasonable (i.e. to make it clearer in code what is going on to improve maintainability). Of course, for protecting from malicious users who may cause your program to behave in unintended ways, I'd say it's not an ideal approach. – Brian May 10 '10 at 18:41
  • 1
    @Brian: for a small application, that may be true. But doing it in code is too inflexible and to greatly subject to error in subsequent releases. If accidental changes are a significant problem, then the database itself needs to prevent them. – John Saunders May 10 '10 at 18:47
  • @Brian, in this case, I'm definitely more concerned about protecting the data from coding mistakes (and just making my intent clear) than malicious users. @John's comments and some of the answers here, though, are making me consider just writing a single repository class (maybe with interfaces, as suggested by Eric), and call it a day. – devuxer May 10 '10 at 18:55
  • @John, this is a pretty small app, but I'm intending some aspects of it to be reused by other programmers. – devuxer May 10 '10 at 18:56

7 Answers7

29

How about a third option, closely related to the first, but using interfaces instead:

public interface IReadRepository {
    public void Select();
}

public interface IWriteRepository {
    public void Insert();
    public void Update();
    public void Delete();
}

// Optional
public interface IRepository : IReadRepository, IWriteRepository {
}

public class Repository : IRepository {
   // Implementation
}

This way the implementation is (or can be) all in one place, and the distinction is made only by which interface you are looking at.

Eric Petroelje
  • 59,820
  • 9
  • 127
  • 177
  • I think this is an interesting pattern, because you can enforce at least a "semantic" kind of security, by giving the objects in your program that "need write access and are cleared for write access" access to the "writeable" interfaces. I would not just think of this as a security issue (because security concerns are a database permission issue), but also as a "minimize how much interface you give various objects within the system", as a good OOP principle. – Warren P May 10 '10 at 18:28
  • But still, unless the domain calls for WriteOnly classes, I would link the 2 interfaces (IWriteRepository: IReadRepository) and that means the inheritance question has shifted to the interfaces. – H H May 10 '10 at 18:33
  • 3
    @Henk - Exactly, you could skip the IWriteRepository and simply have the IRepository interface define the write methods. This would give you the advantages of his second design, but without requiring him to split the implementation between two classes. – Eric Petroelje May 10 '10 at 18:37
  • @stakx: that is what i meant, it depends on the domain. – H H May 10 '10 at 19:20
4

(Edit: I think Eric Petroelje offers a very nice interface-based solution in his answer. I would probably vote for his suggestion, first of all.)

From your two choices, I would clearly vote for design #2.

With design #1, I think it doesn't make sense to have a "read-only" class that internally isn't read-only at all:

  1. The read-only class is "heavier" than it needs to be.

  2. Anyone can derive from your read-only class and then call any of the base class' modification methods. At the very least, with design #1, you ought to make the read-only class sealed.

With design #2, it's much clearer than the read-only class is a reduced version (base class) of the full-featured class, or phrased differently.

stakx - no longer contributing
  • 83,039
  • 20
  • 168
  • 268
1

First let me admit that I am making some assumptions about what you might intend doing. If this misses the point then let me know.

I am not sure how usefull the classes would be in either of your two options. I assume you would have calling code that would use an instance of a readonly repository and at othertimes an instance of a read/write repository, but the interfaces do not match so you would have to differenciate in your code anyway?

It might be better to provide a common interface and then throw exceptions if you try to write to the repository when it is readony and have your code handle the exceptions.

Chris Taylor
  • 52,623
  • 10
  • 78
  • 89
1

I would say design #2, but then you ought to change the name of the ReadOnlyRepository to something like ReadRepository.

Inheritance defines an IS-A relation between the classes, ans saying 'a ReadWriteRepository is a ReadOnlyRepository' doesn't sound logical. But 'ReadWriteRepository is a ReadingRepository' does.

H H
  • 263,252
  • 30
  • 330
  • 514
0

I would certainly say that design 2 is the strongest. If you want to have a read-only implementation, it does not need to know anything about writing. It makes perfect sense to extend the read-only implementation with the insert, update, and delete methods. I also think this design is the one that conforms the most to the Open-Closed principle.

driis
  • 161,458
  • 45
  • 265
  • 341
0

The answer by Eric means SOLID Principle ISP. It´s so easy and basic to use.

Gabriel Simas
  • 567
  • 9
  • 15
0

I would suggest having a base class ReadableFoo, with a sealed derived class ImmutableFoo whose constructor takes a ReadableFoo, and a possibly-inheritable derived class MutableFoo, and possibly a class ReadableShadowFoo whose constructor would take a ReadableFoo (which may or may not be mutable), but which would act as a read-only wrapper to it.

supercat
  • 77,689
  • 9
  • 166
  • 211