Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create trait for BaseIdRepository #39

Open
nagirrab opened this issue Sep 3, 2014 · 3 comments
Open

Create trait for BaseIdRepository #39

nagirrab opened this issue Sep 3, 2014 · 3 comments

Comments

@nagirrab
Copy link

nagirrab commented Sep 3, 2014

Hi,

would it be possible to make it such that the BaseIdRepository class implements some kind of trait which includes all the methods not in BaseIdQueries (e.g. findAll, deleteAll)? I am looking to use a cake pattern to allow easy mocking of the repositories for testing and it would be significantly easier if I could refactor them to be used as follows:

   trait UserRepositoryComponent {
     def userRepository: UserRepository

     trait UserRepository extends BaseIdRepository[User,...]
   }
   trait PostgresUserRepositoryComponent {
     val userRepository = new PostgresUserRepository
     class PostgresUserRepository extends BaseIdRepositoryImpl[User,...]
   }
   trait TestUserRepositoryComponent {
     val userRepository = mock[UserRepository]
   }

It seems cleaner to mock the trait than having to mock the final class instead.

What do you think? I'm happy to work on a PR for this if you think it acceptable.

Cheers,
Hugh

@bambuchaAdm
Copy link
Contributor

It's nice idea. The simple reason why we hadn't got that interface is that unicorn wasn't initially built using cake pattern.

As i assume you want to extract abstract interface for BaseIdRepository ? I don't like suffix Impl as is in your example, but it's only naming.

@nagirrab
Copy link
Author

I'll try to find some time to make the changes in the next couple of weeks. Do you have any preference on the name other than Impl?

Also related - as it stands the underlying TableQuery is protected in the base repository. I found this slightly awkward when for example doing joins across tables which each had a separate repository. Do you have any objection to making this field public?

@bambuchaAdm
Copy link
Contributor

In scala standard library they use Like suffix for abstract implementation.

We are aware problem with corss-repository joins. I this should be escalate as different issue where we would discuss abut the best solution for this.

Abut making TableQuery public. It think it is potential breaking Demeter rule and could easily increase coupling or leaking abstraction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants