Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

The only thing that I think if slightly leaky is updating an existing entity.

It isn't the only leak you have:

public interface ISession : IDisposable

This is a leaky abstraction. You have a specific implementation in mind - one that implements IDisposable, and you're leaking this specific implementation into the ISession interface.

That said, I don't really see what the abstract repository earns you. I've seen people wrap EF with a repository/unit-of-work so that they could "swap the ORM" as needed (if that's ever needed), and I can understand why someone really willing to decouple their service layer from the underlying data access technology would want to do that.

But you're purposely leaking IQueryable<T> into your service layer, which defeats the purpose of the repository abstraction.

Why bother? The added complexity doesn't seem to simplify extending the application. I think your service layer can afford to play with EF directly - DbContext is a repository and a unit-of-work. No need to look further.


What you've identified as a leak with the updating of entities, stems from the fact that your service layer is passing entities to your Web/UI layer - you need a seam here: entities belong with EF and the data-related stuff, not in your UI.

I have to admit I don't do much Web development, but in WPF I'd make that service layer work with ViewModel objects, and be able to "translate" a ViewModel into one or more entities. This "seam" completely decouples your front-end from your back-end. Yes, often the ViewModel class will look very similar to the entity class it's wrapping (see this question this question for a WPF example), but it will only expose what the UI is interested in, so you can keep fields like Id, DateInserted and DateUpdated in the service and data layers and not be bothered with them in the UI layer. I don't think that concern is WPF-specific.

With the service layer performing the "translation" between ViewModel and entity types, you have a perfectly logical spot to perform your validations in, exactly where you wanted to do them in the first place.

The only thing that I think if slightly leaky is updating an existing entity.

It isn't the only leak you have:

public interface ISession : IDisposable

This is a leaky abstraction. You have a specific implementation in mind - one that implements IDisposable, and you're leaking this specific implementation into the ISession interface.

That said, I don't really see what the abstract repository earns you. I've seen people wrap EF with a repository/unit-of-work so that they could "swap the ORM" as needed (if that's ever needed), and I can understand why someone really willing to decouple their service layer from the underlying data access technology would want to do that.

But you're purposely leaking IQueryable<T> into your service layer, which defeats the purpose of the repository abstraction.

Why bother? The added complexity doesn't seem to simplify extending the application. I think your service layer can afford to play with EF directly - DbContext is a repository and a unit-of-work. No need to look further.


What you've identified as a leak with the updating of entities, stems from the fact that your service layer is passing entities to your Web/UI layer - you need a seam here: entities belong with EF and the data-related stuff, not in your UI.

I have to admit I don't do much Web development, but in WPF I'd make that service layer work with ViewModel objects, and be able to "translate" a ViewModel into one or more entities. This "seam" completely decouples your front-end from your back-end. Yes, often the ViewModel class will look very similar to the entity class it's wrapping (see this question for a WPF example), but it will only expose what the UI is interested in, so you can keep fields like Id, DateInserted and DateUpdated in the service and data layers and not be bothered with them in the UI layer. I don't think that concern is WPF-specific.

With the service layer performing the "translation" between ViewModel and entity types, you have a perfectly logical spot to perform your validations in, exactly where you wanted to do them in the first place.

The only thing that I think if slightly leaky is updating an existing entity.

It isn't the only leak you have:

public interface ISession : IDisposable

This is a leaky abstraction. You have a specific implementation in mind - one that implements IDisposable, and you're leaking this specific implementation into the ISession interface.

That said, I don't really see what the abstract repository earns you. I've seen people wrap EF with a repository/unit-of-work so that they could "swap the ORM" as needed (if that's ever needed), and I can understand why someone really willing to decouple their service layer from the underlying data access technology would want to do that.

But you're purposely leaking IQueryable<T> into your service layer, which defeats the purpose of the repository abstraction.

Why bother? The added complexity doesn't seem to simplify extending the application. I think your service layer can afford to play with EF directly - DbContext is a repository and a unit-of-work. No need to look further.


What you've identified as a leak with the updating of entities, stems from the fact that your service layer is passing entities to your Web/UI layer - you need a seam here: entities belong with EF and the data-related stuff, not in your UI.

I have to admit I don't do much Web development, but in WPF I'd make that service layer work with ViewModel objects, and be able to "translate" a ViewModel into one or more entities. This "seam" completely decouples your front-end from your back-end. Yes, often the ViewModel class will look very similar to the entity class it's wrapping (see this question for a WPF example), but it will only expose what the UI is interested in, so you can keep fields like Id, DateInserted and DateUpdated in the service and data layers and not be bothered with them in the UI layer. I don't think that concern is WPF-specific.

With the service layer performing the "translation" between ViewModel and entity types, you have a perfectly logical spot to perform your validations in, exactly where you wanted to do them in the first place.

Source Link
Mathieu Guindon
  • 75.5k
  • 18
  • 194
  • 467

The only thing that I think if slightly leaky is updating an existing entity.

It isn't the only leak you have:

public interface ISession : IDisposable

This is a leaky abstraction. You have a specific implementation in mind - one that implements IDisposable, and you're leaking this specific implementation into the ISession interface.

That said, I don't really see what the abstract repository earns you. I've seen people wrap EF with a repository/unit-of-work so that they could "swap the ORM" as needed (if that's ever needed), and I can understand why someone really willing to decouple their service layer from the underlying data access technology would want to do that.

But you're purposely leaking IQueryable<T> into your service layer, which defeats the purpose of the repository abstraction.

Why bother? The added complexity doesn't seem to simplify extending the application. I think your service layer can afford to play with EF directly - DbContext is a repository and a unit-of-work. No need to look further.


What you've identified as a leak with the updating of entities, stems from the fact that your service layer is passing entities to your Web/UI layer - you need a seam here: entities belong with EF and the data-related stuff, not in your UI.

I have to admit I don't do much Web development, but in WPF I'd make that service layer work with ViewModel objects, and be able to "translate" a ViewModel into one or more entities. This "seam" completely decouples your front-end from your back-end. Yes, often the ViewModel class will look very similar to the entity class it's wrapping (see this question for a WPF example), but it will only expose what the UI is interested in, so you can keep fields like Id, DateInserted and DateUpdated in the service and data layers and not be bothered with them in the UI layer. I don't think that concern is WPF-specific.

With the service layer performing the "translation" between ViewModel and entity types, you have a perfectly logical spot to perform your validations in, exactly where you wanted to do them in the first place.

lang-cs

AltStyle によって変換されたページ (->オリジナル) /