2
\$\begingroup\$

This is my first asp.net core web api and i did followed the Repository pattern but i don't know whether this approach is good practice.

public class TrainerRepo : IRepoTrainer
{
    private readonly IConfiguration _config;

    public TrainerRepo(IConfiguration config)
    {
        _config = config;
    }

    public IDbConnection dbConnection
    {
        get
        {
            return new SqlConnection(_config.GetConnectionString("MyConnString"));
        }
    }

    public List<Trainer> GetTrainers()
    {
        string sql = "select trainer_id,trainer_name from trainer";
        using(IDbConnection conn = dbConnection)
        {
            conn.Open();
            var res = conn.Query<Trainer>(sql).ToList<Trainer>();
            conn.Close();
            return res;
        }

    }

    public int NewTrainer(Trainer trainer)
    {
        string sql = "insert into trainer ( trainer_name) values ( @name)";
        using(IDbConnection conn = dbConnection)
        {
            conn.Open();
            var res = conn.Execute(sql, param: new
            {
                //id = trainer.trainer_id,
                name = trainer.trainer_name
            });
            conn.Close();
            return res;
        }
    }

    public async Task<int> updateTrainer(Trainer trainer)
    {
        string sql = "update trainer set trainer_name = @tname where trainer_id = @tid";
        using(IDbConnection conn = dbConnection)
        {
            conn.Open();
            var res = await conn.ExecuteAsync(sql, param: new { tname = trainer.trainer_name, tid = trainer.trainer_id });
            conn.Close();
            return res;
        }
    }

    public int deleteTrainer(int id)
    {
        string sql = "delete from trainer where trainer_id=@trainerId";
        using(IDbConnection conn = dbConnection)
        {
            conn.Open();
            var res =  conn.Execute(sql, param: new { trainerId = id });
            conn.Close();
            return res;
        }
    }

}

IRepoTrainer interface

public interface IRepoTrainer
{
    List<Trainer> GetTrainers();
    int NewTrainer(Trainer trainer);
    Task<int> updateTrainer(Trainer trainer);
    int deleteTrainer(int id);
}

my problem is that reuse of following code block, is it good practice ?

 using(IDbConnection conn = dbConnection)
   {
    conn.Open();
    //
    conn.Close();
    return res;
   }
\$\endgroup\$
  • \$\begingroup\$ You could make the repo disposable. Open the connection only if it is closed, and close it when the repo is disposed. \$\endgroup\$ – Nkosi Oct 4 '18 at 17:01
  • \$\begingroup\$ It has been suggested in docs to not pass IConfiguration around for injection. Try creating a IConnectionFactory or just have a connection injected into the repo \$\endgroup\$ – Nkosi Oct 4 '18 at 17:04
  • \$\begingroup\$ Could you provide me some example code please ! @Nkosi \$\endgroup\$ – Lakshan Oct 4 '18 at 17:40
  • \$\begingroup\$ @Nkosi I wouldn't do it (the repo disposable) becuase then you cannot use this repository with DI easily. You would need to get a new instance each time. It's better when the APIs control connections themselfes. It would also be difficult to use multiple ORMs per repository. \$\endgroup\$ – t3chb0t Oct 4 '18 at 18:07
  • \$\begingroup\$ @Nkosi - those comments sound like they are worth writing in an answer (you'll probably get some votes, too!). Please do so - comments are supposed to be for improving the question, not answering it! \$\endgroup\$ – Toby Speight Oct 4 '18 at 18:12
1
\$\begingroup\$

It has been suggested in docs to not pass IConfiguration around for injection. Try creating a IConnectionFactory or just have a connection injected directly into the repo

Something like

public interface IDbConnectionFactory {
   IDbConnection CreateConnection();
}

and its implementation for your class would look something like this

public class SqlConnectionFactory : IDbConnectionFactory {
    private readonly string dbConnectionString;

    public SqlConnectionFactory(string dbConnectionString) {
        this.dbConnectionString = dbConnectionString;
    }

    public IDbConnection CreateConnection() {
        return new SqlConnection(this.dbConnectionString);
    }
}

it would be registered in startup

var connectionString = Configuration.GetConnectionString("MyConnString");
services.AddScoped<IDbConnectionFactory, SqlConnectionFactory>(_ => new SqlConnectionFactory(connectionString));

//...

and injected

public class TrainerRepo : IRepoTrainer {
    private readonly IDbConnectionFactory factory;

    public TrainerRepo(IDbConnectionFactory factory) {
        this.factory = factory;
    }

    public List<Trainer> GetTrainers() {
        string sql = "select trainer_id,trainer_name from trainer";
        using(IDbConnection connection = factory.CreateConnection()) {
            connection.Open();
            var res = connection.Query<Trainer>(sql).ToList<Trainer>();
            connection.Close();
            return res;
        }
    }

    //...

The repeated code can also be generalized

T Invoke<T>(Func<IDbConnection, T> func) {
    using(IDbConnection connection = factory.CreateConnection()) {
        connection.Open();
        var res = func(connection);
        connection.Close();
        return res;
    }
}

async Task<TResult> InvokeAsync<TResult>(Func<IDbConnection, Task<TResult>> func) {
    using(IDbConnection connection = factory.CreateConnection()) {
        connection.Open();
        var res = await func(connection);
        connection.Close();
        return res;
    }
}

Which changes the previous examples to

public List<Trainer> GetTrainers() {
    string sql = "select trainer_id,trainer_name from trainer";
    return Invoke(connection => connection.Query<Trainer>(sql).ToList<Trainer>());
}


public Task<int> updateTrainer(Trainer trainer) {
    string sql = "update trainer set trainer_name = @tname where trainer_id = @tid";
    return  InvokeAsync(connection => connection.ExecuteAsync(sql, param: new { tname = trainer.trainer_name, tid = trainer.trainer_id }));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.