C# compiler error with explicit type vars sent to method with ref objects (converting from VB.net)

There are some fundamental things in this method which make me believe you should spend more time refactoring vs direct translation. Restoring type safety is one of them (VB.Net made it easier to hide some poor type safety choices in a file where you have Option Strict Off for a couple modules), but this also REALLY scares me:

//real function will accept sql string

Functions like that tend to cause HUGE security problems, as well as other issues, especially when you also have a bunch of arguments for output values. If you're not well-versed in SQL Injection, NOW is the time to learn about it. You must also provide a way to include input data for the SQL command that is completely separate from the SQL string itself, or you'll eventually find yourself in big trouble.

This code needs some serious refactoring, not just simple conversion!

I suggest refactoring around a method like this:

public class DB
{
    private static string ConnectionString {get;} = "connection string here";

    private static IEnumerable<IDataRecord> getdata(string sql, Action<SqlParameterCollection> addParameters)
    {
        using (var cn = new SqlConnection(ConnectionString))
        using (var cmd = new SqlCommand(sql, cn))
        {
            addParameters(cmd.Parameters);

            cn.Open();
            using (var rdr = cmd.ExecuteReader())
            {
                while (rdr.Read())
                    yield return rdr;
                rdr.Close();
            }
        }
    }
}

Notice the method is private; this is because we're not done building the class yet. Once you create this class, and remove the old getdata() method, everywhere that currently calls this method will turn into a compiler error. This is good; it gives you an easy way to find all those places you had poor code of this type.

So now we start looking at the new compiler errors. Each one will represent a place where you used to call getdata(). There's probably other code nearby to build up the SQL string. You want to move each of these sections to a new static method in the DB class.

One of those methods might look something like this:

public static IDataRecord MyNewDataMethod(int ID)
{
    string SQL = "SELECT ... WHERE ID = @ID";

    return getdata(SQL, p => {
        p.Add("@ID", SqlDbType.Int).Value = ID;
    }).FirstOrDefault();
}

But we can (and should) take this a step further. Typically, these results will represent objects of some type. After all, they had to come from a table, or at least a set of related tables. If you don't already have a class for each of these things, you probably should. These classes should have static methods named something like FromDataRecord(), which accept an IDataRecord or DataRow as input and return the class type as output. They are Factory methods. And now we update the methods to look more like this:

public static MyObjectType MyNewDataMethod(int MyObjectTypeID)
{
    string SQL = "SELECT ... WHERE ID = @ID";

    return getdata(SQL, p => {
        p.Add("@ID", SqlDbType.Int).Value = MyObjectTypeID;
    }).Select(MyObjectType.FromDataRecord).FirstOrDefault();
}

Here's another example that might return several records:

public static IEnumerable<MyObjectType> MyNewDataMethod(string SearchKey)
{
    string SQL = "SELECT ... WHERE SearchColumn = @SearchKey + '%'";

    return getdata(SQL, p => {
        p.Add("@SearchKey", SqlDbType.NVarChar, 80).Value = SearchKey;
    }).Select(MyObjectType.FromDataRecord);
}

If you find you have a lot of these methods, you can convert the private getdata() method to protected, put it in it's own class library project in the solution, and use separate public classes in the same project that can still access that method to divide the data access into logical areas.

I agree with Joel's sentiments; throw this code away rather than trying to salvage it. It's garbage.

If you add a reference to the Nuget package Dapper, your life will get a lot easier. With Dapper, you write SQL and it maps to objects for you. It looks something like this:

using(var c = new SqlConnection(connection_string_here){

   var person = c.QueryFirst<(string Na, string Ad, int Ag)>("SELECT name, address, age FORM person WHERE id = @id", new { id = 123 });

}

There's a lot going on in this, so i'll unpack it:

• The first line just creates a database conenction in a using, so it will be disposed of. You don't need to bother about anything else; Dapper will open the connection, use it, close it

• The second line has some parts:

• var person = - like Dim x = 1 in VB, var declares a variable that is type-detected by the compiler from whatever type is on the right hand side

• c.QueryFirst<(string Na, string Ad, int Ag)> - QueryFirst is a Dapper extension method that runs a select query and pulls the first row. Dapper maps the query columns to the type you give in angle brackets. Here I've given a ValueTuple which is a way to get the C# compiler to "fake" a class for you based on the ValueTuple class. A discussion about how it works is a bit out of scope, but suffice to say when the compiler encounters (string X, string Y, int Z) it transforms behind the scenes into something that you can refer to as an object with those named/typed properties. Suffice to say, when all is done, you'll be able to say person.Na or person.Ad in your code

• "SELECT name, address, age FORM person WHERE id = @id" - is a parameterized SQL. It looks up a person with some ID and pulls their data out in that order, name, address, age. The order in this case is important because AFIAWA dapper maps ValueTuples positionally, not by name. This is different to other things (example later) where it does map by name. The tuple has name/address/age, so the query pulls them in the same order

• new { id = 123 } - is creating a C# anonymous type, a sort of internal-only compiler generated class (different to a valuetuple) that has no name, but does have a property called id with value 123. Dapper will scan your SQL string looking for parameters, and find one called @id, so it will pull the value 123 out of the supplied anonymous type's id property (name based this time, not positional)

If you have a class Person lying around, as you probably should if you're doing any reasonable amount of database-to-c#-and-back-again work, then the call can look like this:

class Person{
    public string Name {get; set;}
    public string Address {get; set;}
    public int Age {get; set;}
}

...

    c.QueryFirst<Person>("SELECT age, name, address FROM ... WHERE id = @i", new { i=123 });

This time we pass a full class Person - Dapper will map the proeprties by name, which is why they're in a different order in the SQL (it could even be SELECT * and dapper will just ignore the 10+ columns in our person table that arent represented by an class property) and it still works. If your SQL names don't match your class names, the simplest thing to do is alias them in the SQL:

c.QueryFirst<Person>("SELECT firstname+' '+lastname as name, ... FROM ... WHERE id = @i", new { i=123 });

I don't think there's an elegant solution - you can keep your 'getdata' method unchanged if you add extra baggage to every method call:

private void test()
{
    string a = null;
    int b = 0;
    string c = null;

    object temp_a = a;
    object temp_b = b;
    object temp_c = c;
    getdata(1, ref temp_a, ref temp_b, ref temp_c);
    a = (string)temp_a;
    b = (int)temp_b;
    c = (string)temp_c;

    MessageBox.Show($"a={a}, b={b}, c={c}");
}

I ended up with the following

Hashtable gd = getData();

string location = (string)gd["location"];
int locationid = (int)gd["locationid"];
string frutata = (string)gd["frutata"];

where getData() just builds a hashtable of objects with the datareader columns.

My end goal was to create a simple callable function w/o a bunch of code to handle return values.

Dapper seems pretty cool and I will def. check that out.