For a project I am working on I have this OwnershipRole record:

public record OwnershipRole
{

    public OwnershipRole()
    {

    }

    public OwnershipRole(Guid id, string title, bool isWithdrawn)
    {
        Id = id;
        Title = title;
        IsWithdrawn = isWithdrawn;
    }

    public Guid Id { get; set; }
    public string Title { get; set; }
    public bool IsWithdrawn { get; set; }
}
}

For getting these details I am using an sql query like so:

    public List<OwnershipRole> GetOwnershipRoles()
    {

        var sql = @"
SELECT ItemID, Title, Status
FROM ItemDetail";


        var data = GetTable(sql);

        return data.Select().Select(dr => new OwnershipRole
        {
            Id = dr.Field<Guid>(0),
            Title = dr.Field<string>(1),
            IsWithdrawn = !dr.Field<bool>(2)
        }).ToList();
    }

    private DataTable GetTable(string sql)
    {
        var rv = new DataTable();
        using var cnn = new SqlConnection(_connectionString);
        using var cmd = new SqlCommand(sql, cnn);
        cnn.Open();

        var da = new SqlDataAdapter(cmd);
        da.Fill(rv);
    
        cnn.Close();
        return rv;
    }

We are grabbing the results and putting them into a data table. The issue I am having however is that the values from the column Status are actually strings. So I am wondering is it possible have a condition of something like:

if (Status == "Withdrawn") {
Status == false

}

I know that won't work but is there a way I can manipulate the values that I get from the Status column in order to fit in with the bool parameter in the OwnershipRole class?

CodePudding user response：

Sure, either:

        var sql = @"
SELECT ItemID, Title, CASE WHEN Status='Withdrawn' then 1 else 0 end IsWithdrawn
FROM ItemDetail";

or

    return data.Select().Select(dr => new OwnershipRole
    {
        Id = dr.Field<Guid>(0),
        Title = dr.Field<string>(1),
        IsWithdrawn = dr.Field<string>(2)=="Withdrawn"?true:false
    }).ToList();

CodePudding user response：

Yes, you can well read string from RDBS and then have bool Status from it (let get rid of GetTable method):

  public List<OwnershipRole> GetOwnershipRoles() {
    using var cnn = new SqlConnection(_connectionString);

    cnn.Open();

    string sql =
      @"SELECT ItemID, 
               Title, 
               Status
          FROM ItemDetail";

    using var cmd = new SqlCommand(sql, cnn);

    var result = new List<OwnershipRole>(); 

    using var reader = cmd.OpenReader();

    while (reader.Read()) { 
      result.Add(new OwnershipRole(
        Guid.Parse(Convert.ToString(reader[0])),
        Convert.ToString(reader[1]),
        Convert.ToString(reader[2]) != "Withdrawn" 
      )); 
    }

    return result; 
  }

Here in the line

 Convert.ToString(reader[2]) != "Withdrawn" 

we read the last field (Status) as a string which we then convert to bool by comparing to "Withdrawn"

CodePudding user response：

From the comments:

The GetTable() method really scares me. It practically forces you to write code that will be horribly vulnerable to sql injection.

How would you improve it?

I'd do something like this:

private DataTable GetTable(string sql, SqlParameter[] paras = null)
{
    var rv = new DataTable();
    using var cnn = new SqlConnection(_connectionString);
    using var cmd = new SqlCommand(sql, cnn);
    using var da = new SqlDataAdapter(cmd);
    if (paras != null && paras.Length > 0)
    {
        cmd.Parameters.AddRange(paras);
    }

    da.Fill(rv);
    return rv;
}

private IEnumerable<IDataRecord> GetRows(string sql, SqlParameter[] paras = null)
{
    var rv = new DataTable();
    using var cnn = new SqlConnection(_connectionString);
    using var cmd = new SqlCommand(sql, cnn);
    if (paras != null && paras.Length > 0)
    {
        cmd.Parameters.AddRange(paras);
    }
    cnn.Open();
    using var rdr = cmd.ExecuteReader();
    while (rdr.Read())
    {
        yield return rdr;
    }
}

private IEnumerable<T> GetItems<T>(string sql, Func<IDataRecord, T> transform, SqlParameter[] paras = null)
{
    var rv = new DataTable();
    using var cnn = new SqlConnection(_connectionString);
    using var cmd = new SqlCommand(sql, cnn);
    if (paras != null && paras.Length > 0)
    {
        cmd.Parameters.AddRange(paras);
    }
    cnn.Open();
    using var rdr = cmd.ExecuteReader();
    while (rdr.Read())
    {
        yield return transform(rdr);
    }
}

The first method works as-is. You don't even need to use the additional argument for simple queries if there really is no parameters. The other two methods work like this:

public IEnumerable<OwnershipRole> GetOwnershipRoles()
{
    var sql = @"
SELECT ItemID, Title, Status
FROM ItemDetail";

    var data = GetRows(sql);
    return data.Select(dr => new OwnershipRole
        {
            Id = (Guid)dr[0],
            Title = (string)dr[1],
            IsWithdrawn = ((string)dr[2])!="Withdrawn"
        });
}

And like this:

public IEnumerable<OwnershipRole> GetOwnershipRoles()
{
    var sql = @"
SELECT ItemID, Title, Status
FROM ItemDetail";

    return GetItems<OwnershipRole>(sql, row => new OwnershipRole
       {
           Id = (Guid)dr[0],
           Title = (string)dr[1],
           IsWithdrawn = ((string)dr[2])!="Withdrawn"
       });
}

Note the return type changes from List to IEnumerable. A good rule of thumb is the keep things as an IEnumerable for as long as possible. This can reduce memory use and set you up to stream values, so in some cases you never need the entire result set in memory at all. Soon we'll want to use IAsyncEnumerable.

When you do really need a list (which is rarer than you might think), you append the ToList() call after calling the method. Especially be careful calling ToList() on the GetRows() function, as it yields the same object on each iteration. Doing that wrong can give you a bunch of records all representing the last row.

Finally, if you don't know how to use SqlParameter objects, STOP and go Google that, because it's a really huge deal.