Generic Repository ... How can I do this?

S

shapper

Hello,

I have been creating a few repositories and all seem the same:

public interface IArticleRepository {
void Create(Article article);
void Delete(Int32 id);
IQueryable<Article> GetAll();
Article GetById(Int32 id);
void Update(Article article);
} // IArticleRepository

And:

public class ArticleRepository : IArticleRepository {
private AppContext context;
public ArticleRepository(ISession session) {
if (session == null)
throw new ArgumentNullException("session");
context = session as AppContext;
} // ArticleRepository

public void Create(Article article) {
context.AddToArticles(article);
} // Create

public void Delete(Int32 id) {
Article article = context.Articles.Where(a => a.Id.Equals
(id)).FirstOrDefault();
context.DeleteObject(article);
} // Delete

public IQueryable<Article> GetAll() {
return context.Articles.OrderBy(a => a.Created).AsQueryable();
} // GetAll

public Article GetById(Int32 id) {
return context.Articles.Where(a => a.Id.Equals
(id)).FirstOrDefault();
} // GetById

public void Update(Article article) {
EntityKey key = context.CreateEntityKey("AppContext.Articles",
article);
object _article;
if (context.TryGetObjectByKey(key, out _article)) {
context.ApplyPropertyChanges(key.EntitySetName, article);
}
} // Update
}

Can I create a base repository based on <T> and having the basic
methods:
Create, Delete, Update, GetById and GetAll?

Or maybe even instead of having GetById and GetAll maybe using some
kind of filter like:
Get(Some filter in linq?? that can be passed) ...

How can I do this?

Thanks,
Miguel
 
P

Peter Duniho

Hello,

I have been creating a few repositories and all seem the same:

[...]
Can I create a base repository based on <T> and having the basic methods:
Create, Delete, Update, GetById and GetAll?

No, not solely given the example you posted.

If you have some way to fix your "AppContext" so that it has a generic
method that can conditionally do the right thing based on the type
parameter, that would be one approach. For example, instead of
AppContext.AddToArticles(), etc., it would have AppContext.AddTo<T>(), and
within would select the appropriate collection based on the type parameter
T (for example, it might look the collection up in a dictionary or simply
execute a switch statement or something).


Alternatively, if you can generalize the LINQ operations by using some
base class or interface that, for example, the "Articles" class (i.e.
whatever the "Articles" property returns) inherits or implements, as well
as a base class or interface that the object class itself inherits or
implements, then you could create a base class that takes advantage of
that, so that the type-specific version only has to add a little more
functionality.

For example, suppose "Articles" and classes like it all implement
IEnumerable<T> (as it appears they do), and the "Article" class and
classes like it implement (my made-up type) IRepositoryObject where we can
find properties like "Id", "Created", etc. Then you can have the
interface and base class like this:

public interface IRepositoryBase<T> {
void Create(T t);
void Delete(Int32 id);
IQueryable<T> GetAll();
T GetById(Int32 id);
void Update(T t);
}

IRepositoryObject
{
Int32 Id { get; }
DateTime Created { get; }
// etc. if necessary
}

public abstract class RepositoryBase<T> : IRepositoryBase<T> where T :
IRepositoryObject
{
protected readonly AppContext context;
private IEnumerable<T> collection;
private string entityKey;

public RepositoryBase(ISession session, IEnumerable<T> collection,
string entityKey) {
if (session == null)
throw new ArgumentNullException("session");
context = session as AppContext;

this.collection = collection;
this.entityKey = entityKey;
}

public abstract void Create(T t);

public void Delete(Int32 id) {
T t = collection.Where(a => a.Id.Equals(id)).FirstOrDefault();
context.DeleteObject(t);
} // Delete

public IQueryable<T> GetAll() {
return collection.OrderBy(a => a.Created).AsQueryable();
} // GetAll

public Article GetById(Int32 id) {
return collection.Where(a => a.Id.Equals(id)).FirstOrDefault();
} // GetById

public void Update(T t) {
EntityKey key = context.CreateEntityKey(entityKey, t);
object _t;
if (context.TryGetObjectByKey(key, out _t)) {
context.ApplyPropertyChanges(key.EntitySetName, t);
}
} // Update
}

Then you can more-easily create the type-specific implementation. For
example:

class ArticleRepository : RepositoryBase<Article>
{
public ArticleRepository(ISession session)
: base(session, ((AppContext)session).Articles,
"AppContext.Articles")
{ }

public void Create(Article article)
{
context.AddToArticles(article);
}
}

Note that your ability to do this depends on whether you are able to
generalize the related objects in this way. But, as you can see, if you
can then the actual implementation for each type-specific class gets much
simpler.

By the way, a couple of comments on style:

-- IMHO, it's poor practice to cast an interface _back_ to some
concrete type. There are unusual cases where this may be acceptable, but
in general if you are dealing with the interface type, it is specifically
because someone's made the design decision that the code using it is or
should be limited to the features found in the interface. Not only is
that "un-abstraction" a maintenance problem, it presents the risk that the
code using the interface could in fact be dealing with a completely
different type, causing a failure later on in the code that "un-abstracts"
the interface.

-- Speaking of said failure, your use of "as" puzzles me. You aren't
doing any verification on the result, which means that if the ISession
turns out not to be an AppContext, the constructor will succeed, but
you'll get a null exception later. You really ought to be casting rather
than using "as"; at least then, if some other code breaks you abstraction,
you get an error that is more specific and immediately understandable
(i.e. an invalid cast exception).

-- Finally, while I don't insist that people follow the convention of
using an underscore for private or protected class members, I definitely
think the underscore doesn't belong on _local_ variable names. I
preserved your convention in my example for the sake of continuity, but if
you find yourself needing to distinguish between a local variable and an
argument, you should do so with something besides an underscore. The
naming convention I generally use (Hungarian) has specific rules for this
(i.e. use a specific qualifier, either the general "T" for "temp" or a
qualifier that is more specific to the usage), but it doesn't matter so
much what convention you so much as you use something that informs the
person reading the code as to what each variable really _is_, rather than
just "this one is different from the other, and oh by the way I named it
the same way most people name private and protected members, or internal
library or language features". :)
Or maybe even instead of having GetById and GetAll maybe using some
kind of filter like:
Get(Some filter in linq?? that can be passed) ...

I don't actually understand this part. It seems orthogonal to the
question of generalizing the class.

Pete
 
S

shapper

On Sep 18, 3:10 am, "Peter Duniho" <[email protected]>
wrote:

Hello Pete,

I followed your example, my code and Googled for info.
I am using EntityFramework and AutoMapper to map between the EF models
and the UI models.

I am trying to create a common repository and different services that
would would make possible to communicate with the database and at the
same time create the mapping.

The class automatically generated by EF is named AppEntities.
I tried to create a UnitOfWork over all EF entities so I have:

public partial class AppEntities : ISession {
// Some Code
}

The problem is that further down in my code GetTable is not
recognized. If I change it to:

public partial class AppEntities : DataContext { }

It is recognized but I get "Partial declarations of 'Lab.AppEntities'
must not specify different base classes"

I am not sure if this method of using partial is the way to go.

In IRepository I also have a Find method so that later I can pass Linq
expressions with some condition to filter records.
I don't know how to implement this and if this is really needed ... It
seemed a good idea.

I am stuck now ...

Here is the code:


using AutoMapper;
using Lab;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;
using System.Data.Linq;
using System.Data.Objects;
using System.Data.Entity;
using System.Data.EntityClient;
using System.Reflection;

namespace Lab {

class Program {
static void Main(string[] args) {
}
}

public interface ISession {
void Commit();
void Rollback();
} // ISession

public partial class AppEntities : DataContext {
public void Commit() {
SaveChanges();
} // Commit
public void Rollback() {
Dispose();
} // Rollback
} // AppEntities

// IRepository
public interface IRepository<T> {

void Create(T o);
void Delete(T o);
//void Find(System.Linq.Expressions.Expression<Func<T, bool>>
predicate);
IQueryable<T> GetAll();
T GetById(int id);
int Update(T o);

} // IRepository

// Repository
public class Repository<T> : IRepository<T> {

private AppEntities context;

public Repository(ISession session) {
if (session == null)
throw new ArgumentNullException("session");
context = session as AppEntities;
}

public void Create(T o) {
var tableName = typeof(T).Name;
context.AddObject(tableName, o);
}

public void Delete(T o) {
var tableName = typeof(T).Name;
context.DeleteObject(o);
}

public IQueryable<T> GetAll() {
var tableName = typeof(T).Name;
return context.GetTable<T>().AsQueryable();
}

public T GetById(int id) {
var tableName = typeof(T).Name;
var key = new EntityKey("AppEntities." + tableName, "Id", id);
var r = (T)context.GetObjectByKey(key);
return r;
}

public int Update(T o) {
var tableName = typeof(T).Name;
var Id = GetPrimaryKeyValue(o);
T currentObject = GetById(Id);
currentObject = o;
context.ApplyPropertyChanges(tableName, currentObject);
return context.SaveChanges();
}

private int GetPrimaryKeyValue(T o) {
int Id = 0;
var properties = typeof(T).GetProperties();
foreach (PropertyInfo p in properties) {
if (string.Compare(p.Name, "Id", true) == 0) {
var r = p.GetValue(o, null).ToString();
Id = int.Parse(r);
break;
}
}
return Id;
}

} // Repository

// Service
public abstract class Service<TModel, T> where TModel : new() where
T : new() {

public Repository<T> _repo;

protected Service(ISession session) {
_repo = new Repository<T>(session);

var tm = Mapper.FindTypeMapFor<T, TModel>();
if (tm == null) {
Mapper.CreateMap<T, TModel>();
}

tm = Mapper.FindTypeMapFor<TModel, T>();
if (tm == null) {
Mapper.CreateMap<TModel, T>();
}
}

public virtual TModel GetById(Int32 id) {
var item = _repo.GetById(id);
var model = Map(item);
return model;
}

public virtual TModel Map(T d) {
return Mapper.Map<T, TModel>(d);
}

} // Service

public class ArticleUI {
public int Id { get; set; }
public String Title { get; set; }
public String Content { get; set; }
}

// ArticleService
public class ArticleService : Service<ArticleUI, Article> {

public ArticleService(ISession session) : base(session) {
}

public List<ArticleUI> GetPublished() {
var result = _repo.Find(a => a.Published == true);
var articles = result.ToList();
return Map(articles);
}

}

}

I appreciate any help ... I am really completely stuck ... I think I
am in the good direction but I don't know how to finish this.

Thank You,
Miguel
 
P

Peter Duniho

I followed your example, my code and Googled for info.
I am using EntityFramework and AutoMapper to map between the EF models
and the UI models.

I am trying to create a common repository and different services that
would would make possible to communicate with the database and at the
same time create the mapping.

The class automatically generated by EF is named AppEntities.

As noted before, when you start running into problems that appear to be
caused by your use of something outside the specific realm of C#, you are
usually going to be much better off finding a forum specific to that area
of .NET and posting your question there.

If you even have to mention "Entity Framework" as part of asking your
question, you have a very difficult problem getting your question answered
here, because those of us unfamiliar with EF have no context, nor any
knowledge of how EF works, nor any knowledge of common idioms and patterns
used to write code with EF.

It may well be that what you're trying to solve is a well-known problem
for people using EF. But if you're the only person in this newsgroup
using it, it will take a LOT more time and effort for other people here
(including me) to get enough information to solve your problem.

That said...
I tried to create a UnitOfWork over all EF entities so I have:

public partial class AppEntities : ISession {
// Some Code
}

The problem is that further down in my code GetTable is not
recognized. If I change it to:

public partial class AppEntities : DataContext { }

It is recognized but I get "Partial declarations of 'Lab.AppEntities'
must not specify different base classes"

Well, where is the rest of the class declared then? Who decided that the
"base class" (interface, actually...the error is really complaining that
in the first partial declaration, no base class was specified but in the
second, DataContext is) should be ISession, rather than DataContext?
Would changing it to DataContext even be useful? Either way, the real
question is whether you can pass an AppEntitites object to the repository
classes, so that you don't have to do a downcast to get to the type you
really want.
I am not sure if this method of using partial is the way to go.

Well, why are you using partial? Is that mandated by EF? Or is it
something you decided upon yourself?
In IRepository I also have a Find method so that later I can pass Linq
expressions with some condition to filter records.
I don't know how to implement this and if this is really needed ... It
seemed a good idea.

Based on the other modifications you made to my previous suggestion (all
of which seem fine, as far as I can tell), I wouldn't expect the Find()
method to be much different. You'd still access the collection by type
name the way your other methods do; then rather than performing a specific
operation or returning the table as IQueryable (for example), you'd just
get the table within the method and call Find() on it using the predicate
passed to the method.

At least, based on the vague question, I think that's the answer. It's
hard to know what to answer when the question is no more detailed than "I
don't know how to do it". If you get to a point where there's some
specific line or lines of code you want to write but for some reason it's
not working, and you can state specifically what about it isn't working
(e.g. error message, incorrect behavior, etc.), that would really help.
Barring that, hopefully the above is helpful enough.

Pete
 
S

shapper

Hi Pete,

I asked in the EF forum but the answer I got was wrong ... I finally
found the problem. Now everything compiles.

About Find what you say is since GetAll() returns all records I could
use for example inside the service:
var result = _repo.GetAll().Where(a => a.Published == true);

Instead of:
var result = _repo.Find(a => a.Published == true);

If this is what you meant I think your approach is better.

About partial I was doing everything fine, well at least now compiles.
The problem was with the advice I was getting in EF where they
answered me as I was using Linq To SQL instead of Linq to Entities.
But I was on a Linq to Entities forum. :)

This said this is my code. I created a simple EF model with only one
Entity: Article. And post also my code.
If you have any suggestion is welcome ... But I think I am making this
right.

using AutoMapper;
using Lab;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;
using System.Data.Linq;
using System.Data.Objects;
using System.Data.Entity;
using System.Data.EntityClient;
using System.Reflection;

namespace Lab {

class Program {
static void Main(string[] args) {
}
}

public interface ISession {
void Commit();
void Rollback();
} // ISession

public partial class AppEntities : ISession {

public void Commit() {
SaveChanges();
} // Commit
public void Rollback() {
Dispose();
} // Rollback

} // AppEntities

// IRepository
public interface IRepository<T> {

void Create(T o);
void Delete(T o);
//void Find(System.Linq.Expressions.Expression<Func<T, bool>>
predicate);
IQueryable<T> GetAll();
T GetById(int id);
int Update(T o);

} // IRepository

// Repository
public class Repository<T> : IRepository<T> {

private AppEntities context;

public Repository(ISession session) {
if (session == null)
throw new ArgumentNullException("session");
context = session as AppEntities;
}

public void Create(T o) {
var tableName = typeof(T).Name;
context.AddObject(tableName, o);
}

public void Delete(T o) {
var tableName = typeof(T).Name;
context.DeleteObject(o);
}

public IQueryable<T> GetAll() {
var tableName = typeof(T).Name;
return context.CreateQuery<T>(tableName).AsQueryable();
}

public T GetById(int id) {
var tableName = typeof(T).Name;
var key = new EntityKey("AppEntities." + tableName, "Id", id);
var r = (T)context.GetObjectByKey(key);
return r;
}

public int Update(T o) {
var tableName = typeof(T).Name;
var Id = GetPrimaryKeyValue(o);
T currentObject = GetById(Id);
currentObject = o;
context.ApplyPropertyChanges(tableName, currentObject);
return context.SaveChanges();
}

private int GetPrimaryKeyValue(T o) {
int Id = 0;
var properties = typeof(T).GetProperties();
foreach (PropertyInfo p in properties) {
if (string.Compare(p.Name, "Id", true) == 0) {
var r = p.GetValue(o, null).ToString();
Id = int.Parse(r);
break;
}
}
return Id;
}

} // Repository

// Service
public abstract class Service<TModel, T> where TModel : new() where
T : new() {

public Repository<T> _repo;

protected Service(ISession session) {
_repo = new Repository<T>(session);

var tm = Mapper.FindTypeMapFor<T, TModel>();
if (tm == null) {
Mapper.CreateMap<T, TModel>();
}

tm = Mapper.FindTypeMapFor<TModel, T>();
if (tm == null) {
Mapper.CreateMap<TModel, T>();
}
}

public virtual TModel GetById(Int32 id) {
var item = _repo.GetById(id);
var model = Map(item);
return model;
}

public virtual TModel Map(T d) {
return Mapper.Map<T, TModel>(d);
}

public virtual IList<TModel> Map(IList<T> d) {
return Mapper.Map<IList<T>, List<TModel>>(d);
}

} // Service

public class ArticleUI {

public int Id { get; set; }
public String Title { get; set; }
public String Content { get; set; }

}

// ArticleService
public class ArticleService : Service<ArticleUI, Article> {

public ArticleService(ISession session) : base(session) {
}

public IList<ArticleUI> GetPublished() {
var query = _repo.GetAll().Where(a => a.Published == true);
var articles = query.ToList();
return Map(articles);
}

}

}

And what EF generates is the following:

[assembly: global::System.Data.Objects.DataClasses.EdmSchemaAttribute
()]

// Original file name:
// Generation date: 18-09-2009 19:05:56
namespace Lab
{

/// <summary>
/// There are no comments for AppEntities in the schema.
/// </summary>
public partial class AppEntities :
global::System.Data.Objects.ObjectContext
{
/// <summary>
/// Initializes a new AppEntities object using the connection
string found in the 'AppEntities' section of the application
configuration file.
/// </summary>
public AppEntities() :
base("name=AppEntities", "AppEntities")
{
this.OnContextCreated();
}
/// <summary>
/// Initialize a new AppEntities object.
/// </summary>
public AppEntities(string connectionString) :
base(connectionString, "AppEntities")
{
this.OnContextCreated();
}
/// <summary>
/// Initialize a new AppEntities object.
/// </summary>
public AppEntities
(global::System.Data.EntityClient.EntityConnection connection) :
base(connection, "AppEntities")
{
this.OnContextCreated();
}
partial void OnContextCreated();
/// <summary>
/// There are no comments for Articles in the schema.
/// </summary>
public global::System.Data.Objects.ObjectQuery<Article>
Articles
{
get
{
if ((this._Articles == null))
{
this._Articles = base.CreateQuery<Article>
("[Articles]");
}
return this._Articles;
}
}
private global::System.Data.Objects.ObjectQuery<Article>
_Articles;
/// <summary>
/// There are no comments for Articles in the schema.
/// </summary>
public void AddToArticles(Article article)
{
base.AddObject("Articles", article);
}
}
/// <summary>
/// There are no comments for Lab.Article in the schema.
/// </summary>
/// <KeyProperties>
/// Id
/// </KeyProperties>
[global::System.Data.Objects.DataClasses.EdmEntityTypeAttribute
(NamespaceName="Lab", Name="Article")]
[global::System.Runtime.Serialization.DataContractAttribute
(IsReference=true)]
[global::System.Serializable()]
public partial class Article :
global::System.Data.Objects.DataClasses.EntityObject
{
/// <summary>
/// Create a new Article object.
/// </summary>
/// <param name="id">Initial value of Id.</param>
/// <param name="content">Initial value of Content.</param>
/// <param name="created">Initial value of Created.</param>
/// <param name="published">Initial value of Published.</
param>
/// <param name="title">Initial value of Title.</param>
/// <param name="updated">Initial value of Updated.</param>
public static Article CreateArticle(int id, string content,
global::System.DateTime created, bool published, string title,
global::System.DateTime updated)
{
Article article = new Article();
article.Id = id;
article.Content = content;
article.Created = created;
article.Published = published;
article.Title = title;
article.Updated = updated;
return article;
}
/// <summary>
/// There are no comments for Property Id in the schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute
(EntityKeyProperty=true, IsNullable=false)]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public int Id
{
get
{
return this._Id;
}
set
{
this.OnIdChanging(value);
this.ReportPropertyChanging("Id");
this._Id =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value);
this.ReportPropertyChanged("Id");
this.OnIdChanged();
}
}
private int _Id;
partial void OnIdChanging(int value);
partial void OnIdChanged();
/// <summary>
/// There are no comments for Property Content in the schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute
(IsNullable=false)]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public string Content
{
get
{
return this._Content;
}
set
{
this.OnContentChanging(value);
this.ReportPropertyChanging("Content");
this._Content =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value, false);
this.ReportPropertyChanged("Content");
this.OnContentChanged();
}
}
private string _Content;
partial void OnContentChanging(string value);
partial void OnContentChanged();
/// <summary>
/// There are no comments for Property Created in the schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute
(IsNullable=false)]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public global::System.DateTime Created
{
get
{
return this._Created;
}
set
{
this.OnCreatedChanging(value);
this.ReportPropertyChanging("Created");
this._Created =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value);
this.ReportPropertyChanged("Created");
this.OnCreatedChanged();
}
}
private global::System.DateTime _Created;
partial void OnCreatedChanging(global::System.DateTime value);
partial void OnCreatedChanged();
/// <summary>
/// There are no comments for Property Excerpt in the schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute()]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public string Excerpt
{
get
{
return this._Excerpt;
}
set
{
this.OnExcerptChanging(value);
this.ReportPropertyChanging("Excerpt");
this._Excerpt =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value, true);
this.ReportPropertyChanged("Excerpt");
this.OnExcerptChanged();
}
}
private string _Excerpt;
partial void OnExcerptChanging(string value);
partial void OnExcerptChanged();
/// <summary>
/// There are no comments for Property Published in the
schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute
(IsNullable=false)]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public bool Published
{
get
{
return this._Published;
}
set
{
this.OnPublishedChanging(value);
this.ReportPropertyChanging("Published");
this._Published =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value);
this.ReportPropertyChanged("Published");
this.OnPublishedChanged();
}
}
private bool _Published;
partial void OnPublishedChanging(bool value);
partial void OnPublishedChanged();
/// <summary>
/// There are no comments for Property Title in the schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute
(IsNullable=false)]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public string Title
{
get
{
return this._Title;
}
set
{
this.OnTitleChanging(value);
this.ReportPropertyChanging("Title");
this._Title =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value, false);
this.ReportPropertyChanged("Title");
this.OnTitleChanged();
}
}
private string _Title;
partial void OnTitleChanging(string value);
partial void OnTitleChanged();
/// <summary>
/// There are no comments for Property Updated in the schema.
/// </summary>

[global::System.Data.Objects.DataClasses.EdmScalarPropertyAttribute
(IsNullable=false)]
[global::System.Runtime.Serialization.DataMemberAttribute()]
public global::System.DateTime Updated
{
get
{
return this._Updated;
}
set
{
this.OnUpdatedChanging(value);
this.ReportPropertyChanging("Updated");
this._Updated =
global::System.Data.Objects.DataClasses.StructuralObject.SetValidValue
(value);
this.ReportPropertyChanged("Updated");
this.OnUpdatedChanged();
}
}
private global::System.DateTime _Updated;
partial void OnUpdatedChanging(global::System.DateTime value);
partial void OnUpdatedChanged();
}
}

Maybe this might be useful for someone.

Thanks,
Miguel
 
P

Peter Duniho

Hi Pete,

I asked in the EF forum but the answer I got was wrong ... I finally
found the problem. Now everything compiles.

About Find what you say is since GetAll() returns all records I could
use for example inside the service:
var result = _repo.GetAll().Where(a => a.Published == true);

Instead of:
var result = _repo.Find(a => a.Published == true);

If this is what you meant I think your approach is better.

Unfortuantely, your original code example wasn't clear about what the
Find() method is supposed to do, as your commented-out declaration had a
return type of "void". But, assuming you want a collection of all items
meeting a specific criteria, and you don't mind making the caller just go
through the GetAll() method, then your first example is fine.

If you want to provide a Find() method as a convenience, it should be
trivial to implement that within the generic class simply by wrapping the
implementation using the call to GetAll(). IMHO, either approach is okay;
the convenience method adds very little, but "very little" isn't the same
as "nothing at all". :)

As for the rest, looks fine to me. But then, as I warned, I don't really
know anything about the Entity Framework. There could be all sorts of
other problems in the code that I didn't notice simply because I'm
unfamiliar with the context. :)

Pete
 
S

shapper

I was looking at your code again and I got a few doubts about a few
things I did on my code. In:

public class Repository<T> : IRepository<T> {
private AppEntities context;

Should I use the following?

Speaking of said failure, your use of "as" puzzles me. You aren't
doing any verification on the result, which means that if the ISession
turns out not to be an AppContext, the constructor will succeed, but
you'll get a null exception later. You really ought to be casting rather
than using "as"; at least then, if some other code breaks you abstraction,
you get an error that is more specific and immediately understandable
(i.e. an invalid cast exception).

I think I am doing the same and you referring to the following:

public Repository(ISession session) {
if (session == null)
throw new ArgumentNullException("session");
context = session as AppEntities;
}

Could you tell me specifically how should I do this?
Finally, while I don't insist that people follow the convention of
using an underscore for private or protected class members, I definitely
think the underscore doesn't belong on _local_ variable names. I
preserved your convention in my example for the sake of continuity, but if
you find yourself needing to distinguish between a local variable and an
argument, you should do so with something besides an underscore

You mean that when I use:
public Repository<T> _repo;

I should use _ when for private and protected fields inside a class
and nothing when it is public?
I think that's it ...

I have been reading some naming conventions but they differ in some
cases.

For example I am using to use Caps for constants like PERIOD but in
some guides I get the information to not use it ...

So sometimes I use a little bit of what I am used ...
 
P

Peter Duniho

I was looking at your code again and I got a few doubts about a few
things I did on my code. In:

public class Repository<T> : IRepository<T> {
private AppEntities context;

Should I use the following?

public class Repository<T> : IRepository<T> {
protected readonly AppEntities context;

No, not with the version you wrote. I put that in there because in order
to support the Create() method, I had to make the Repository<T> class
abstract, to be overridden by type-specific classes. Those type-specific
classes needed access to the "context" member variable, hence the
"protected" instead of "private". I added the "readonly" mainly to
protect against the derived classes from messing with the variable; you
may actually want to include that in your own Repository<T> class simply
because it's good form to put "readonly" on member fields that you know
should never change after object construction.

The "protected" would be superfluous in any case. You can leave that as
"private" in your own version, assuming you have no plans to inherit the
Repository said:
I think I am doing the same and you referring to the following:

public Repository(ISession session) {
if (session == null)
throw new ArgumentNullException("session");
context = session as AppEntities;
}

Could you tell me specifically how should I do this?

Just change this:

context = session as AppEntities;

to this:

context = (AppEntities)session;

As I mentioned before, I don't like the casting in any case. But if
you're going to do it, do it so that it fails right away if someone does
it wrong.
You mean that when I use:
public Repository<T> _repo;

I should use _ when for private and protected fields inside a class
and nothing when it is public?

Yes. Except that you should not have a public field anyway. Things that
you are inclined to make as public fields should instead be abstracted
with a property; this allows you to a) provide read-only semantics as
needed, and b) if you allow it to be writable, to do validation as
appropriate.

The one exception to the "no public fields" rule would be if the field is
"readonly". Which in this case might be appropriate.
I think that's it ...

I have been reading some naming conventions but they differ in some
cases.

For example I am using to use Caps for constants like PERIOD but in
some guides I get the information to not use it ...

So sometimes I use a little bit of what I am used ...

I wouldn't worry too much about most of it. But the underscore as part of
member names is so widely used to mean some kind of non-public member, I
think it's worth following no matter what other conventions you're using.

Pete
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top