Is this Repository Design Pattern Valid and efficient?

I am using Dapper ORM in my Data access Layer, but I think analysis of the code below doesn't depend upon any specific ORM. What should be the single common place throughout the entire Data Access Layer to initialise the Connection String?

Base Abstract Class

abstract class Repository
    {
        protected static IDbConnection _db = new SqlConnection(ConfigurationManager.ConnectionStrings["GuideDB"].ConnectionString);

      //should I use Constructor here
    }

A Generic Repository Interface

public interface IRepository
    {
        IEnumerable GetAll();
        T GetItem(int id);
        T Create(T item);
        bool Update(T item);
        bool Delete(int id);
        void Reset();
    }

Concrete class

```
public class StateRepository :Repository, IRepository
{

private static ICollection _states = PopulateStates();// doing this valid ??
private static object _lock = new object();

#region Public Methods

public IEnumerable GetAll()
{
lock (_lock)
{
return _states.ToArray();
}
}

public IEnumerable GetAllStatesByCountryId(short id)
{
lock (_lock)
{
return _states.Where(s => s.CountryId == id).ToArray();
}
}

public State GetItem(int id)
{
lock (_lock)
{
return _states.FirstOrDefault(s => s.Id == id);
}
}

public State Create(State state)
{
lock (_lock)
{
if (state == null)
{
throw new ArgumentNullException("state");
}

state.Id = (short)(_states.Any() ? _states.Max(s => s.Id) + 1 : 1);
_states.Add(state);
return state;
}
}

public bool Update(State item)
{
lock (_lock)
{

I think your abstract class should "implement" the IRepository interface, because otherwise you will always have to put both the interface and the base class in your inheritance list. Also, I think you should name your Repository -> SqlRepository, since all the code in the base class is coupled to Sql. As @Heslacher pointed out, you should inject your connection, because keeping the static connection might cause problems. What would happen if one of my Repository implementation would dispose the connection? All your repositories would be down.

abstract class SqlRepository : IRepository
{
    protected IDbConnection Connection { get; set; }

    protected SqlRepository(string connectionString)
    {
        Connection = new SqlConnection(connectionString);
    }

    abstract IEnumerable GetAll();
    abstract T GetItem(int id);
    abstract T Create(T item);
    abstract bool Update(T item);
    abstract bool Delete(int id);
    abstract void Reset();
}

If your application runs on one instance, your repository would be okay. But since it puts all database entities in cache (your collection) and never returns to the database unless you specify it, you have a problem. Example : Client A (On an X machine) loads the database in cache, Client B (on a Y machine) does the same then adds a State to the repository. Client A calls GetAll, what should be the expected output? Client A wouldn't have the new State. And if Client B calls Reset the new State was never persisted, so it is lost.

I don't think you should use the ICollection, each of your call should go to the database, so you are sure you have the latest version of your data. If you remove it, I don't think you need to handle thread safety anymore.

StackExchange Code Review Q#63933, answer score: 9