4
\$\begingroup\$

I have two database tables UsersDownload, Non-UsersDownload. There is also two model object and two service that take in the object and insert the data into the correct table. Is this the best architecture for this? or is there a better way to handle this? Would it be better to use an interface?

public class UserLog
{
    public string CustomerId { get; set; }
    public bool? IsMobile { get; set; }
    public int? DeviceId { get; set; }
} 
public class NonUserLog
{
    public string DeviceId { get; set; }
    public bool? IsMobile { get; set; }
    public string AppType { get; set; }
} 

insert using Store Procedure

 internal ResultStatus UsersDownload(UserLog userLog)
        {
            queryName = "usp_Insert_UserLog";
            SQLHelper sql = new SQLHelper(conn);
            ResultStatus rs = new ResultStatus();
            SqlParameter[] parms = new SqlParameter[] {
                            new SqlParameter("@CustomerID", SqlDbType.NVarChar, 50),
                            new SqlParameter("@IsMobile", SqlDbType.Bit),
                        new SqlParameter("@DeviceId", SqlDbType.Int)
            };

            parms[0].Value = guidelineLog.CustomerId;
            parms[1].Value = guidelineLog.IsMobile;
            parms[2].Value = guidelineLog.DeviceId;

            using (SqlDataReader dataReader = sql.ExecuteReaderStoreProcedure(queryName, parms))
            {
                rs.ResultCode = 1;
                rs.ResultMessage = "Success";
                dataReader.Close();
            }

            return rs;
        }
 internal ResultStatus NonUsersDownload(NonUserLog nonUserLog)
        {
            queryName = "usp_Insert_NonUserLog";
            SQLHelper sql = new SQLHelper(conn);
            ResultStatus rs = new ResultStatus();
            SqlParameter[] parms = new SqlParameter[] {
                            new SqlParameter("@DeviceId", SqlDbType.NVarChar, 50),
                            new SqlParameter("@IsMobile", SqlDbType.Bit),
                        new SqlParameter("@AppType", SqlDbType.NVarChar)
            };

            parms[0].Value = guidelineLog.DeviceId;
            parms[1].Value = guidelineLog.IsMobile;
            parms[2].Value = guidelineLog.AppType;

            using (SqlDataReader dataReader = sql.ExecuteReaderStoreProcedure(queryName, parms))
            {
                rs.ResultCode = 1;
                rs.ResultMessage = "Success";
                dataReader.Close();
            }

            return rs;
        }

service

public ResultStatus InsertUsersDownload(UserLog userLog)
        {
            return glDb.UsersDownload(userLog);
        }

        public ResultStatus InsertNonUsersDownload(NonUserLog nonUserLog)
        {
            return glDb.NonUsersDownload(nonUserLog);
        }
\$\endgroup\$
1

1 Answer 1

3
\$\begingroup\$

It'll add more work if you use interface since your models are different, only one property (Mobile) is the common one, and you'll add Reflection to the equation as well. Even if you used a generic method, you'll add extra work to it. So, in your case, your best approach would be to use overloads (which you've already done) and keep it simple, but you've over-done it. Rather than just duplicating the method and change the model, you'll need to set a private base method, which is going to be used as call back. The other overloads will be set to each process respectively, then it'll callback the base method.

// UserLog overload
internal ResultStatus DownloadsLog(UserLog log)
{
    var parameters = new SqlParameter[]
    {
        new SqlParameter { ParameterName = "@CustomerID", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = log.CustomerId },
        new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = log.IsMobile },
        new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.Int, Value = log.DeviceId }
    };

    return DownloadsLog("usp_Insert_UserLog", parameters);
}

// NonUserLog overload
internal ResultStatus DownloadsLog(NonUserLog log)
{
    var parameters = new SqlParameter[]
    {
        new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = log.DeviceId },
        new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = log.IsMobile },
        new SqlParameter { ParameterName = "@AppType", SqlDbType = SqlDbType.NVarChar, Value = log.AppType }
    };

    return DownloadsLog("usp_Insert_NonUserLog", parameters);
}

// base
private ResultStatus DownloadsLog(string procedureName, SqlParameter[] parameters)
{
    using (var dataReader = new SQLHelper(conn).ExecuteReaderStoreProcedure(procedureName, parameters))
    {
        return new ResultStatus
        {
            ResultCode = 1,
            ResultMessage = "Success"
        };
    }
}

as you can see, the UserLog and NonUserLog overloads are there just to set the parameters with the procedure name. Then, I just callback the base method.

Update

This is another approach where you define the parameters and the store procedure name in the model itself, kinda like gathering everything related into one box.

public interface ISqlDbLog
{
    string GetSqlProcedureName();

    SqlParameter[] GetSqlParameters();
}

public class UserLog : ISqlDbLog
{
    public string CustomerId { get; set; }

    public bool? IsMobile { get; set; }

    public int? DeviceId { get; set; }

    public string GetSqlProcedureName() => "usp_Insert_UserLog";

    public SqlParameter[] GetSqlParameters()
    {
        return new SqlParameter[]
        {
            new SqlParameter { ParameterName = "@CustomerID", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = CustomerId },
            new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = IsMobile },
            new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.Int, Value = DeviceId }
        };
    }
}

public class NonUserLog : ISqlDbLog
{
    public string DeviceId { get; set; }

    public bool? IsMobile { get; set; }

    public string AppType { get; set; }

    public string GetSqlProcedureName() => "usp_Insert_NonUserLog";

    public SqlParameter[] GetSqlParameters()
    {
        return new SqlParameter[]
        {
            new SqlParameter { ParameterName = "@DeviceId", SqlDbType = SqlDbType.NVarChar, Size = 50, Value = DeviceId },
            new SqlParameter { ParameterName = "@IsMobile", SqlDbType = SqlDbType.Bit, Value = IsMobile },
            new SqlParameter { ParameterName = "@AppType", SqlDbType = SqlDbType.NVarChar, Value = AppType }
        };
    }
}

class Program
{
    internal static ResultStatus DownloadsLog(ISqlDbLog log)
    {
        return DownloadsLog(log.GetSqlProcedureName(), log.GetSqlParameters());
    }

    static void Main(string[] args)
    {
        var log = new UserLog
        {
            CustomerId = "Test Customer",
            DeviceId = 5,
            IsMobile = true
        };

        var result = DownloadsLog(log);

    }
}

the advantage of this approach is that you have defined the data mapping within the model, which would be easier to navigate. The other thing, you're also defining the correct SqlDbType mapping. So, instead of using AddWithValue, you will pass the correct datatype, and it's already defined in the model. You can make a generic method where you use Reflection to auto-generate the SqlParameters, all you have to do is to change GetSqlParameters() to :

public SqlParameter[] GetSqlParameters()
{
    return this.GetType()
        .GetProperties()
        .Where(x => x.GetValue(this, null) != null)
        .Select(x => new SqlParameter($"@{x.Name}", x.GetValue(this, null)))
        .ToArray();
}

you can use it as an extension, if you have a wider usage (like applying it to other models as well). like this :

public static class SqlDbExtensions
{
    public static SqlParameter[] ToSqlParameterArray<T>(this T model) where T : ISqlDbLog
    {
        return model.GetType()
            .GetProperties()
            .Where(x => x.GetValue(model, null) != null)
            .Select(x => new SqlParameter($"@{x.Name}", x.GetValue(model, null)))
            .ToArray();

    }
}

This will be applied on all classes that implement ISqlDbLog. This is an insurance. So, with this, you will not need the GetSqlParameter() method in each class nor the ISqlDbLog interface. but you still need ProcedureName.

I hope this would add more helpful thoughts to your work.

\$\endgroup\$
12
  • \$\begingroup\$ So I would have to send the procedure name to the method is that something common that people do is sending the procedure name? \$\endgroup\$
    – Jefferson
    Commented Jan 22, 2020 at 21:53
  • \$\begingroup\$ @Jefferson yes, and don't worry about it, because it's private and will not be used outside this class scope, unless you expose it. \$\endgroup\$
    – iSR5
    Commented Jan 22, 2020 at 22:01
  • \$\begingroup\$ would it help if I have a flag in the NonUserLog model? Something like IsNonUser? \$\endgroup\$
    – Jefferson
    Commented Jan 23, 2020 at 12:54
  • \$\begingroup\$ Is there a way to have just one service? \$\endgroup\$
    – Jefferson
    Commented Jan 23, 2020 at 13:38
  • \$\begingroup\$ @Jefferson check the update. \$\endgroup\$
    – iSR5
    Commented Jan 23, 2020 at 22:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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