I use the Repository/Service design pattern in my projects and I have found something that might be a bit redundant.  Am I writing any unnecessary code?With that in mind, here is my structure:public class Competition{    public int Id { get; set; }    [Required] public string UserId { get; set; }    public string UserName { get; set; }    public DateTime DateCreated { get; set; }    public DateTime? DateModified { get; set; }    [Required] public string Name { get; set; }}This is an example POCO class, but as you will see I am using Generics so it can be anything:internal interface IRepository<TEntity> : IDisposable where TEntity : class{    IList<TEntity> GetAll();    TEntity Get(int id);    void Save(TEntity model);    void Delete(int id);}My interface implements IDisposable and allows any class object to be set as TEntity:internal class CompetitionRepository : IRepository<Competition>{    private readonly string userId;    private readonly CompetitionProvider provider;    public CompetitionRepository(string userId) : this(userId, DefaultConnection)    {    }    public CompetitionRepository(string userId, string connectionString)    {        this.userId = userId;        this.provider = new CompetitionProvider(connectionString);    }    public IList<Competition> GetAll()    {        return provider.Get(this.userId);    }    public Competition Get(int id)    {        return GetAll().Where(model => model.Id == id).SingleOrDefault();    }    public void Save(Competition model)    {        provider.Save(model);    }    public void Delete(int id)    {        provider.Delete(id);    }    public void Dispose()    {        provider.Dispose();    }}My repository which implements IRepository,  specifying Competition as TEntity.  This repository queries my database hence the provider.In this repository, I only do the minimal of what is required to query my database:public class CompetitionService : IRepository<Competition>{    private readonly string userId;    private readonly IRepository<Competition> repository;    public CompetitionService(string userId) : this(userId, DefaultConnection)    {    }    public CompetitionService(string userId, string connectionString)    {        this.userId = userId;        this.repository = new CompetitionRepository(this.userId, connectionString);    }    public IList<Competition> GetAll()    {        return this.repository.GetAll();    }    public Competition Get(int id)    {        return this.repository.Get(id);    }    public void Save(Competition model)    {        if (model.Id > 0)        {            model.UserId = userId;            model.DateCreated = DateTime.UtcNow;        } else        {            model.DateModified = DateTime.UtcNow;        }        this.repository.Save(model);    }    public void Delete(int id)    {        this.repository.Delete(id);    }    public void Dispose()    {        this.repository.Dispose();    }}Now this is my service. For example, you can see the Save method assigns values depending on whether we are creating or updating. A Service class can have anything in there, for example performing an uploads or whatever is not related to data source.The Repository is there to interact with the data source only and no other code is present. This allows for swapping out the repository with another that queries a different data source.My problem is that I have noticed that, although the service has code that modifies and corrects the values before they are then saved via the repository, the structure is always very similar to the repository, i.e. it has:GetAll()Get()Save()Delete()I have implemented IRepository<Competition> as well.  The service may have other public methods, but it doesn't matter because, in my controllers, I would call it like this://// POST: /Competitions/Create[HttpPost]public ActionResult Create(Competition model){    try    {        using (var service = new CompetitionService(User.Identity.GetUserId()))        {            service.Save(model);        }        return RedirectToAction(Index);    }    catch    {        return View(model);    }}Do you think it is good practice or do you think I am over-killing it a bit?Do you think I should implement IRepository on the service?In the more extreme case, do you think I should even bother with a service?(NB: I am reluctant to remove the service because sometimes there is a lot of code that needs to go in there that I don't want clogging up my controllers)
Repository/Service Design Pattern
c#;design patterns
You are on the right track. The separation of business logic (services) and data-access logic (repositories) is a good thing, I strongly recommend this. I created a generic repository implementing the IRepository<TEntity> interface. It's slightly different than yours, but you get the point (I have a BaseEntity constraint, you can also use class for this):public class EfRepository<TEntity> : IRepository<TEntity>    where TEntity : BaseEntity, new(){    private readonly IDbContext _context;    private IDbSet<TEntity> _entities;    public EfRepository(IDbContext context)    {        _context = context;    }    private IDbSet<TEntity> Entities    {        get { return _entities ?? (_entities = _context.Set<TEntity>()); }    }    public TEntity GetById(object id)    {        return Entities.Find(id);    }    public void Insert(TEntity entity)    {        Entities.Add(entity);        _context.SaveChanges();    }    public void Update(TEntity entity)    {        _context.SaveChanges();    }    public void Delete(int id)    {        // Create a new instance of an entity (BaseEntity) and add the id.        var entity = new TEntity                     {                         ID = id                     };        // Attach the entity to the context and call the delete method.        Entities.Attach(entity);        Delete(entity);    }    public void Delete(TEntity entity)    {        Entities.Remove(entity);        _context.SaveChanges();    }    public IList<TEntity> Table    {        get { return Entities.ToList(); }    }}This greatly reduces the amount of duplicate code when you would use specific repositories. I use this in my services like:public class ProductService : IProductService{    private readonly IRepository<Product> _productRepository;    public ProductService(IRepository<Product> productRepository)    {        _productRepository = productRepository;    }    public bool AddProduct(Product product)    {        // Validate etc.        _productRepository.Insert(product);        // return validation result.        return true;    }}As you may have noticed I use an IoC framework to inject the dependencies.