-2

I have this function on my C# project

public DataTable access2dt()
    {

        string myConnectionString = "Provider=Microsoft.ACE.OLEDB.12.0;Data Source=test.accdb";
        using (var con = new OleDbConnection(myConnectionString))
        {
            con.Open();

            using (var cmd = new OleDbCommand("EXEC OUTBOUND_FILTER",con))
            {
                cmd.Parameters.AddWithValue("prmORIGINCODE", "BDO");
                cmd.Parameters.AddWithValue("prmORIGIN", "\"*\"");
                cmd.Parameters.AddWithValue("prmSERVICECODE", "REG15");
                cmd.Parameters.AddWithValue("prmDESTCODE", "AMI");
                cmd.Parameters.AddWithValue("prmDESTINATION", "\"*\"");
                using (OleDbDataReader rdr = cmd.ExecuteReader())

                {
                    DataTable myTable = new DataTable();
                    myTable.Load(rdr);
                    return myTable;
                }


            }

        }
    }

the function above is for execute query object in Ms Access with some parameter, the code just works for my project but i have problem with "code style".

I want to change the code so while i have another query WITH another different parameter i don't need to rewrite that code (see the parameter, query name and db file name is "hard-coded" to function).

Any suggestion and C# code will be helpfull.

reynd
  • 21
  • 1
  • 4
  • So you want this code to be able to execute any query, with any number of parameters? Then add parameters like `string query, Dictionary queryParameters`, or better even `List`, or then why not just let the caller issue an `OleDbCommand` by themselves? – CodeCaster Jul 28 '16 at 14:52
  • 2
    @Matias just because the question mentions "code style" doesn't mean it belongs at Code Review. – CodeCaster Jul 28 '16 at 14:54
  • 4
    I don't think that this question would be a good fit for Code Review, as OP's specifically asking a question: "I want to be able to change X in my code dynamically, how do I do that?". He/She's not asking "That's my code, do you find anything I can improve here?" – Henrik Ilgen Jul 28 '16 at 14:54
  • yes, i can use variables and pass it to the function, but what about query parameters? the parameter is more than one, and then parameter name and parameter value is two different value/variables i think. – reynd Jul 28 '16 at 14:55
  • Yes, so you want callers of this method execute an arbitrary query with an arbitrary number of parameters. Then simply let the callers instantiate and execute an `OleDbCommand` themselves, and stick the connectionstring in config. – CodeCaster Jul 28 '16 at 14:59
  • @MatiasCicero Please note that there is **no** Close Reason that says "Belongs on Code Review". If it is actually Off-Topic on SO then close it for the relevant Close Reason. You can always suggest moving to Code Review, but the 2 are separate actions and should be treated as such. – Kaz Jul 28 '16 at 15:02

2 Answers2

0

You're right that you will want to refactor this to make it more generalized. One thing that does not change frequently will be that connection string. Perhaps you can abstract that into a DataStore class. Then you can have an ExecuteCommand method that takes the command name and parameters:

public class DataStore
{
    private string _connectionStr;

    public DataStore(string connectionStr)
    {
        this._connectionStr = connectionStr;
    }

    public DataTable ExecuteCommand(string commandText, IDictionary<string,string> parameters)
    {
        using (var connection = new OleDbConnection(this._connectionStr))
        {
            connection.Open();
            using (var cmd = new OleDbCommand(commandText, con))
            {
                foreach (var pair in parameters)
                {
                    cmd.Parameters.AddWithValue(pair.Key, pair.Value);
                }

                using (var reader = cmd.ExecuteReader())
                {
                    var table = new DataTable();
                    table.Load(reader);
                    return table;
                }
            }
        }
    }
}

...but this still has some limitations, like that each parameter value would have to be a string. You could change parameters to be an IDictionary<string, object>, but then you'd lose nice type safety.

Stepping back, you might imagine that lots of developers have the exact same struggle with wrapping database access in an abstraction layer and might have already solved this problem. Indeed, things like Dapper and Entity Framework can do this sort of thing for you.

See How to use Entity framework for MS Access database for some tips on how you can use Entity Framework with MS Access. Or you can seek out any other sort of ORM (Object/Relation Mapper)

Community
  • 1
  • 1
Jacob
  • 77,566
  • 24
  • 149
  • 228
-2
public DataTable access2dt(string filename, string namaQuery, IDictionary<string,string> keyValue)
{
    string myConnectionString = "Provider=Microsoft.ACE.OLEDB.12.0;Data Source=test.accdb";
    using (var con = new OleDbConnection(myConnectionString))
    {
        con.Open();

        using (var cmd = new OleDbCommand("EXEC OUTBOUND_FILTER",con))
        {
            foreach (var d in keyValue)
            {
                cmd.Parameters.AddWithValue(d.Key, d.Value);
            }

            using (OleDbDataReader rdr = cmd.ExecuteReader())
            {
                DataTable myTable = new DataTable();
                myTable.Load(rdr);
                return myTable;
            }
        }
    }
}
Jacob
  • 77,566
  • 24
  • 149
  • 228
suulisin
  • 1,414
  • 1
  • 10
  • 17
  • 1
    Don't answer unclear questions and especially don't dump "fixed" code, explain what you changed and why you think that helps the OP fix the original problem. – CodeCaster Jul 28 '16 at 14:57
  • I have searching about IDictionary sample code, this give me idea but I'm still confused about where I would put that code, but thanks for answer, i think next time i will post my (code) result to Code Review Section. – reynd Jul 28 '16 at 15:19
  • thanks if you still have problem contact me @ suulisindiyaka@stoicteam.com so we can discuss it – suulisin Jul 28 '16 at 15:21