Is invoking one repository from another a code smell?

Question

I recently came accross an old webapp project which is using a repository layer between domain and persistance layer. However, the repository layer has been used weirdly.

One example is the blog section where posts and comments are persisted. A Post has many Comments.

public class Post {
    //..
    private List<Comment> comments;
    //..
}

Post and Comment reside in seperate tables. The post and it's comments are mapped using an intermediate table.

|---------------------|------------------|
|      post_id        |     comment_id   |
|---------------------|------------------|
|         200         |        854       |
|---------------------|------------------|
|         200         |        931       |
|---------------------|------------------|
|        .....        |        ...       |
|---------------------|------------------|

The objects are persisted through PostRepository's save() method.

public class PostRepository {

    public void save(Post post) {
        // db stuff here
        // save it
        String postId = // get the postId

        List<String> commentIds = (new CommentRepository()).save(post.getComments());

        for(String commentId : commentIds) {
            saveMapping(postId, commentId);
        }
    }

    private void saveMapping(String postId, String commentId) {
        // insert in the mapping table
    }
}

I may be mistaken though, but to me it seems this is a violation of SRP, as PostRepository has been assgined to call the CommentRepository. This also makes both PostRepository and CommentRepository aware of each other's existance. Isn't this a code smell?

What refactorings would you recommend?

Answer 1

The real question is: why have a separate repository for the Comments? A repository mediates between the business logic and some kind of storage or data source. But this Comment repo sits between another repo and the storage. Isn't the way in which Comments are stored just an implementation detail of the Post repository?

It's possible that the answer is no, and that you do need a separate Comment repository. This indicates that parts of the business logic handles Comments independently from their parent Post, and that your Post entity maybe should not contain a list of Comments. Instead, comments could directly know about their parent posts. E.g. this design allows you to update the Post without having to re-save all existing comments on that Post.

If you have a separate Comment repo and the Post repository needs to use it, this should be an explicit dependency. I.e. the new CommentRepository() call is quite dubious. In most cases the repository will be defined as an interface, and you shouldn't directly instantiate an implementing class.

If we keep your original design of the Posts, I'd consider writing the repository like this:

interface CommentRepository {
  void save(Comment comment);
  ...
}

interface PostRepository {
  void save(Post post);
}

class PostRepositoryForDatabase implements PostRepository {
  private CommentRepository commentRepo;

  public PostRepositoryForDatabase(CommentRepository commentRepo) {
    this.commentRepo = commentRepo;
  }

  @Override
  public void save(Post post) {
    ... // save post body & metadata
    for (Comment c : post.getComments()) {
      commentRepo.save(c);
      saveMapping(post, c);
    }
  }

  private void saveMapping(Post post, Comment comment) { ... }
}

Using a separate repository like this is mostly fine, because that dependency is now explicit. This still isn't great (because the Post/Comment design in your domain model and in your database is a bit unusual), but such aspects are much harder to fix.

If this were a green field, a typical post/comment design with repositories might look like:

class Post { ... }
class Comment { Post post; ... }

interface PostRepository {
  void save(Post post);
  void saveComment(Comment comment);
  ...
}

class PostRepositoryForDatabase implements PostRepository {
  @Override
  public void save(Post post) {
    ...
  }

  @Override
  public void saveComment(Comment comment) {
    ...
  }
}

With a database schema

posts(id, ...)
comments(id, post_id → posts(id), ...)

I.e. your mapping table is unnecessary unless the relation Posts to Comments is many to many. For this one to many relation, the post_id foreign key can be part of the comments table.

It is not strictly necessary to have separate repositories for Posts and Comments as I'd consider that repository as an abstraction over the same data source. You often see separate classes in C# (that inherit from a generic Repository<T> interface) but that's a language-specific convenience related to features like reflection, LINQ, and extension methods.

In my alternative design, how would the business logic add a comment to a post? Just create a comment for that post and save it:

Comment c = new Comment(post, user, body);
repo.saveComment(c);

If we need a list of all comments on a post, the repository should provide an appropriate method:

interface PostRepository {
  ...
  List<Comment> getCommentsForPost(Post p);
}

I.e. the Post entity would not expose a method like getComments().