2
\$\begingroup\$

As you will see from the code below, I have a base model.

public class BasePDV
{
    public int CaseID { get; set; }
    public string CustContactTel { get; set; }
    public string CustMobileTel { get; set; }
    public string PropertyType { get; set; }
    public string PremisesStatus { get; set; }
    public int LockType { get; set; }
    public int AccessInfo { get; set; }
    public string V1 { get; set; }
    public string V2 { get; set; }
    public int V2a { get; set; }
    public string V3 { get; set; }
    public string V4 { get; set; }
    public string V4a { get; set; }
    public string V5 { get; set; }
    public string V6 { get; set; }
    public string V7 { get; set; }
    public string V7a { get; set; }
    public string V8 { get; set; }
    public string V8a { get; set; }
    public string V9 { get; set; }
    public string V9a { get; set; }
    public string V10 { get; set; }
    public int V10a { get; set; }
    public string V11 { get; set; }
    public string V11a { get; set; }
    public string V12 { get; set; }
    public string VulnerableOther { get; set; }
    public string ContactType { get; set; }
    public string NINumber { get; set; }
    public string ContactDate { get; set; }
    public string CustDOB { get; set; }
    public double? PayAmount { get; set; }
    public double? PayAmount2 { get; set; }
    public int PayMethod { get; set; }
    public string ReceiptNo { get; set; }
    public double? DirectPayAmount { get; set; }
    public bool COT { get; set; }
    public string TitleCode { get; set; }
    public string OtherTitle { get; set; }
    public string FirstName { get; set; }
    public string MiddleInitials { get; set; }
    public string SurName { get; set; }
    public string COTProofType { get; set; }
    public string COTDate { get; set; }
    public string PropRespMtrRead { get; set; }
    public string PropRespMtrBRead { get; set; }
    public string PrevAdd1 { get; set; }
    public string PrevAdd2 { get; set; }
    public string PrevAdd3 { get; set; }
    public string PrevAdd4 { get; set; }
    public string PrevPostcode { get; set; }
    public string FwdName { get; set; }
    public string FwdAdd1 { get; set; }
    public string FwdAdd2 { get; set; }
    public string FwdAdd3 { get; set; }
    public string FwdAdd4 { get; set; }
    public string FwdPostcode { get; set; }
    public bool PayArranged { get; set; }
    public string PayStartDate { get; set; }
    public double? PayArrAmount { get; set; }
    public string Frequency { get; set; }
    public string PayArrDetail { get; set; }
    public bool PPMAgreed { get; set; }
    public string PPMBookedDate { get; set; }
    public string PPMHHBookedTime { get; set; }
    public string EmploymentStatus { get; set; }
    public double? RecoveryRate { get; set; }
    public string PPMApptComments { get; set; }
    public bool PPMArranged { get; set; }
    public string PPMAppointmentDate { get; set; }
    public string PPMDetail { get; set; }
    public string AuthCode { get; set; }
    public string DateOfLeaving { get; set; }
    public string VacantPropComments { get; set; }
    public string CustComplaintCode { get; set; }
    public string DisputeComments { get; set; }
    public string IncidentDets { get; set; }
    public string PDVComments { get; set; }
    public bool Dissatisfaction { get; set; }
    public string DissatisfactionComments { get; set; }
    public bool HealthSafety { get; set; }
    public string ContactCode { get; set; }
    public string WhoSpokenTo { get; set; }
    public string HealthSafetyComments { get; set; }
    public string InfoNotes { get; set; }
    public bool EnergyTrust { get; set; }
    public string EnergyTrustComments { get; set; }
    public string VisitDate1 { get; set; }
    public string VisitTime { get; set; }
    public string VisitDate2 { get; set; }
    public string VisitTime2 { get; set; }
    public string ClientComments { get; set; }
    public string PDVNote { get; set; }

    public virtual List<SqlParameter> GetCommandParameters()
    {
        return new List<SqlParameter>
        {
            new SqlParameter("@CaseID", CaseID),
            new SqlParameter("@CustContactTel", CustContactTel),
            new SqlParameter("@CustMobileTel",CustMobileTel),
            new SqlParameter("@PropertyType", PropertyType),
            new SqlParameter("@PremisesStatus", PremisesStatus),
            new SqlParameter("@LockType", LockType),
            new SqlParameter("@AccessInfo", AccessInfo),
            new SqlParameter("@V1", V1),
            new SqlParameter("@V2", V2),
            new SqlParameter("@V2a", V2a),
            new SqlParameter("@V3", V3),
            new SqlParameter("@V4", V4),
            new SqlParameter("@V4a", V4a),
            new SqlParameter("@V5", V5),
            new SqlParameter("@V6", V6),
            new SqlParameter("@V7", V7),
            new SqlParameter("V7a", V7a),
            new SqlParameter("@V8", V8),
            new SqlParameter("@V8a", V8a),
            new SqlParameter("@V9", V9),
            new SqlParameter("@V9a", V9a),
            new SqlParameter("@V10", V10),
            new SqlParameter("@V10a", V10a),
            new SqlParameter("@V11", V11),
            new SqlParameter("@V11a", V11a),
            new SqlParameter("@V12", V12),
            new SqlParameter("@VulnerableOther", VulnerableOther),
            new SqlParameter("@ContactType", ContactType),
            new SqlParameter("@NINumber", NINumber),
            new SqlParameter("@CustDOB", ContactDate),
            new SqlParameter("@ContactDate", CustDOB),
            new SqlParameter("@PayAmount", PayAmount),
            new SqlParameter("@PayAmount2", PayAmount2),
            new SqlParameter("@PayMethod", PayMethod),
            new SqlParameter("@ReceiptNo", ReceiptNo),
            new SqlParameter("@DirectPayAmount", DirectPayAmount),
            new SqlParameter("@COT", COT),
            new SqlParameter("@TitleCode", TitleCode),
            new SqlParameter("@OtherTitle", OtherTitle),
            new SqlParameter("@FirstName", FirstName),
            new SqlParameter("@MiddleInitials", MiddleInitials),
            new SqlParameter("@SurName", SurName),
            new SqlParameter("@COTProofType", COTProofType),
            new SqlParameter("@COTDate", COTDate),
            new SqlParameter("@PropRespMtrRead", PropRespMtrRead),
            new SqlParameter("@PropRespMtrBRead", PropRespMtrBRead),
            new SqlParameter("@PrevAdd1", PrevAdd1),
            new SqlParameter("@PrevAdd2", PrevAdd2),
            new SqlParameter("@PrevAdd3", PrevAdd3),
            new SqlParameter("@PrevAdd4", PrevAdd4),
            new SqlParameter("@PrevPostcode", PrevPostcode),
            new SqlParameter("@FwdName", FwdName),
            new SqlParameter("@FwdAdd1", FwdAdd1),
            new SqlParameter("@FwdAdd2", FwdAdd2),
            new SqlParameter("@FwdAdd3", FwdAdd3),
            new SqlParameter("@FwdAdd4", FwdAdd4),
            new SqlParameter("@FwdPostcode", FwdPostcode),
            new SqlParameter("@PayArranged", PayArranged),
            new SqlParameter("@PayStartDate", PayStartDate),
            new SqlParameter("@PayArrAmount", PayArrAmount),
            new SqlParameter("@Frequency", Frequency),
            new SqlParameter("@PayArrDetail", PayArrDetail),
            new SqlParameter("@PPMAgreed", PPMAgreed),
            new SqlParameter("@PPMBookedDate", PPMBookedDate),
            new SqlParameter("@PPMHHBookedTime", PPMHHBookedTime),
            new SqlParameter("@EmploymentStatus", EmploymentStatus),
            new SqlParameter("@RecoveryRate", RecoveryRate),
            new SqlParameter("@PPMApptComments", PPMApptComments),
            new SqlParameter("@PPMArranged", PPMArranged),
            new SqlParameter("@PPMAppointmentDate", PPMAppointmentDate),
            new SqlParameter("@PPMDetail", PPMDetail),
            new SqlParameter("@AuthCode", AuthCode),
            new SqlParameter("@DateOfLeaving", DateOfLeaving),
            new SqlParameter("@VacantPropComments", VacantPropComments),
            new SqlParameter("@CustComplaintCode", CustComplaintCode),
            new SqlParameter("@DisputeComments", DisputeComments),
            new SqlParameter("@IncidentDets", IncidentDets),
            new SqlParameter("@PDVComments", PDVComments),
            new SqlParameter("@Dissatisfaction", Dissatisfaction),
            new SqlParameter("@DissatisfactionComments", DissatisfactionComments),
            new SqlParameter("@HealthSafety", HealthSafety),
            new SqlParameter("@Contact_Code", ContactCode),
            new SqlParameter("@WhoSpokenTo", WhoSpokenTo),
            new SqlParameter("@HealthSafetyComments", HealthSafetyComments),
            new SqlParameter("@InfoNotes", InfoNotes),
            new SqlParameter("@EnergyTrust", EnergyTrust),
            new SqlParameter("@EnergyTrustComments", EnergyTrustComments),
            new SqlParameter("@VisitDate1", VisitDate1),
            new SqlParameter("@VisitTime", VisitTime),
            new SqlParameter("@VisitDate2", VisitDate2),
            new SqlParameter("@VisitTime2", VisitTime2)
        };
    }
}

Then I have another class that inherits from the base class.

public class PDV : BasePDV
{
    public object Timestamp { get; set; }
    public bool ClearedFunds { get; set; }
    public bool ClearedFunds2 { get; set; }

    public override List<SqlParameter> GetCommandParameters()
    {
        var rtnList = base.GetCommandParameters();
        rtnList.Add(new SqlParameter("@PDVTimestamp", Timestamp));
        rtnList.Add(new SqlParameter("@ClearedFunds", ClearedFunds));
        rtnList.Add(new SqlParameter("@ClearedFunds2", ClearedFunds2));

        return rtnList;
    }
}

The method GetCommandParameters is used for saving the data, can I use the same style of method but for loading the data as I currently do it like so;

public PDV LoadPDV(int caseID)
    {
        var pdv = new PDV();

        using (SqlConnection cn = new SqlConnection(SQLHelper.ConnectionString))
        {
            using (SqlCommand cmd = new SqlCommand("prcUMGetPDVDetails", cn))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.AddWithValue("@CaseID", caseID);

                cn.Open();

                using (SqlDataReader dr = cmd.ExecuteReader())
                {
                    while (dr.Read())
                    {
                        pdv.Timestamp = dr["PDVStamp"];

                        //Property Details
                        pdv.CustContactTel = dr["CustContactTel"].ToString();
                        pdv.CustMobileTel = dr["CustMobileTel"].ToString();
                        pdv.PropertyType = dr["PropertyType"].ToString();
                        pdv.PremisesStatus = dr["PremisesStatus"].ToString();
                        pdv.LockType = string.IsNullOrEmpty(dr["LockType"].ToString()) ? 0 : Convert.ToInt16(dr["LockType"]);
                        pdv.AccessInfo = string.IsNullOrEmpty(dr["AccessInfo"].ToString()) ? 0 : Convert.ToInt16(dr["AccessInfo"]);

                        //Vulnerability Details
                        pdv.V1 = dr["V1"].ToString();
                        pdv.V2 = dr["V2"].ToString();
                        pdv.V2a = string.IsNullOrEmpty(dr["V2a"].ToString()) ? 0 : Convert.ToInt16(dr["V2a"]);
                        pdv.V3 = dr["V3"].ToString();
                        pdv.V4 = dr["V4"].ToString();
                        pdv.V4a = dr["V4a"].ToString();
                        pdv.V5 = dr["V5"].ToString();
                        pdv.V6 = dr["V6"].ToString();
                        pdv.V7 = dr["V7"].ToString();
                        pdv.V8 = dr["V8"].ToString();
                        pdv.V8a = dr["V8a"].ToString();
                        pdv.V9 = dr["V9"].ToString();
                        pdv.V9a = dr["V9a"].ToString();
                        pdv.V10 = dr["V10"].ToString();
                        pdv.V10a = string.IsNullOrEmpty(dr["V10a"].ToString()) ? 0 : Convert.ToInt32(dr["V10a"]);
                        pdv.V11 = dr["V11"].ToString();
                        pdv.V11a = dr["V11a"].ToString();
                        pdv.V12 = dr["V12"].ToString();
                        pdv.VulnerableOther = dr["VulnerableOther"].ToString();

                        //Contact Details
                        pdv.ContactCode = dr["ContactType"].ToString();
                        pdv.NINumber = dr["NINumber"].ToString();
                        pdv.ContactDate = dr["ContactDate"].ToString();
                        pdv.CustDOB = dr["CustDOB"].ToString();

                        //Payment Details
                        pdv.PayAmount = dr["PayAmount"] == DBNull.Value ? Convert.ToDouble(null) : Convert.ToDouble(dr["PayAmount"]);
                        pdv.ClearedFunds = dr["ClearedFunds"] == DBNull.Value ? false : Convert.ToBoolean(dr["ClearedFunds"]);
                        pdv.PayAmount2 = dr["PayAmount2"] == DBNull.Value ? Convert.ToDouble(null) : Convert.ToDouble(dr["PayAmount2"]);
                        pdv.ClearedFunds2 = dr["ClearedFunds2"] == DBNull.Value ? false : Convert.ToBoolean(dr["ClearedFunds2"]);
                        pdv.PayMethod = string.IsNullOrEmpty(dr["PayMethod"].ToString()) ? 0 : Convert.ToInt32(dr["PayMethod"]);
                        pdv.ReceiptNo = dr["ReceiptNo"].ToString();
                        pdv.DirectPayAmount = dr["DirectPayAmount"] == DBNull.Value ? Convert.ToDouble(null) : Convert.ToDouble(dr["DirectPayAmount"]);

                        //Change Of Tenancy Details
                        pdv.COT = dr["COT"] == DBNull.Value ? false : Convert.ToBoolean(dr["COT"]);
                        pdv.TitleCode = dr["TitleCode"].ToString();
                        pdv.OtherTitle = dr["OtherTitle"].ToString();
                        pdv.FirstName = dr["FirstName"].ToString();
                        pdv.MiddleInitials = dr["MiddleInitials"].ToString();
                        pdv.SurName = dr["SurName"].ToString();
                        pdv.COTProofType = dr["COTProofType"].ToString();
                        pdv.COTDate = dr["COTDate"].ToString();
                        pdv.PropRespMtrRead = dr["PropRespMtrRead"].ToString();
                        pdv.PropRespMtrBRead = dr["PropRespMtrBRead"].ToString();
                        pdv.PrevAdd1 = dr["PrevAdd1"].ToString();
                        pdv.PrevAdd2 = dr["PrevAdd2"].ToString();
                        pdv.PrevAdd3 = dr["PrevAdd3"].ToString();
                        pdv.PrevAdd4 = dr["PrevAdd4"].ToString();
                        pdv.PrevPostcode = dr["PrevPostcode"].ToString();
                        pdv.FwdName = dr["FwdName"].ToString();
                        pdv.FwdAdd1 = dr["FwdAdd1"].ToString();
                        pdv.FwdAdd2 = dr["FwdAdd2"].ToString();
                        pdv.FwdAdd3 = dr["FwdAdd3"].ToString();
                        pdv.FwdAdd4 = dr["FwdAdd4"].ToString();
                        pdv.FwdPostcode = dr["FwdPostcode"].ToString();

                        //Payment Arrangement Details
                        pdv.PayArranged = dr["PayArranged"] == DBNull.Value ? false : Convert.ToBoolean(dr["PayArranged"]);
                        pdv.PayStartDate = dr["PayStartDate"].ToString();
                        pdv.PayArrAmount = dr["PayArrAmount"] == DBNull.Value ? Convert.ToDouble(null) : Convert.ToDouble(dr["PayArrAmount"]);
                        pdv.Frequency = dr["Frequency"].ToString();
                        pdv.PayArrDetail = dr["PayArrDetail"].ToString();

                        //PPM Details
                        pdv.PPMAgreed = dr["PPMAgreed"] == DBNull.Value ? false : Convert.ToBoolean(dr["PPMAgreed"]);
                        pdv.PPMBookedDate = dr["PPMBookedDate"].ToString();
                        pdv.PPMHHBookedTime = dr["PPMHHBookedTime"].ToString();
                        pdv.EmploymentStatus = dr["EmploymentStatus"].ToString();
                        pdv.RecoveryRate = dr["RecoveryRate"] == DBNull.Value ? Convert.ToDouble(null) : Convert.ToDouble(dr["RecoveryRate"]);
                        pdv.PPMApptComments = dr["PPMApptComments"].ToString();

                        //Meter Removal Appointment
                        pdv.PPMArranged = dr["PPMArranged"] == DBNull.Value ? false : Convert.ToBoolean(dr["PPMArranged"]);
                        pdv.PPMAppointmentDate = dr["PPMAppointmentDate"].ToString();
                        pdv.PPMDetail = dr["PPMDetail"].ToString();
                        pdv.AuthCode = dr["AuthCode"].ToString();

                        //Vacant Property Details
                        pdv.DateOfLeaving = dr["DateOfLeaving"].ToString();
                        pdv.VacantPropComments = dr["VacantPropComments"].ToString();

                        //Dispute Details
                        pdv.CustComplaintCode = dr["CustComplaintCode"].ToString();
                        pdv.DisputeComments = dr["DisputeComments"].ToString();
                        pdv.IncidentDets = dr["IncidentDets"].ToString();

                        //Additional Details
                        pdv.PDVComments = dr["PDVComments"].ToString();
                        pdv.Dissatisfaction = dr["Dissatisfaction"] == DBNull.Value ? false : Convert.ToBoolean(dr["Dissatisfaction"]);
                        pdv.DissatisfactionComments = dr["DissatisfactionComments"].ToString();
                        pdv.HealthSafety = dr["HealthSafety"] == DBNull.Value ? false : Convert.ToBoolean(dr["HealthSafety"]);
                        pdv.ContactCode = dr["Contact_Code"].ToString();
                        pdv.WhoSpokenTo = dr["WhoSpokenTo"].ToString();
                        pdv.HealthSafetyComments = dr["HealthSafetyComments"].ToString();
                        pdv.InfoNotes = dr["InfoNotes"].ToString();
                        pdv.EnergyTrust = dr["EnergyTrust"] == DBNull.Value ? false : Convert.ToBoolean(dr["EnergyTrust"]);
                        pdv.EnergyTrustComments = dr["EnergyTrustComments"].ToString();

                        //Visit Details
                        pdv.VisitDate1 = dr["VisitDate1"].ToString();
                        pdv.VisitTime = dr["VisitTime"].ToString();
                        pdv.VisitDate2 = dr["VisitDate2"].ToString();
                        pdv.VisitTime2 = dr["VisitTime2"].ToString();

                        //Other Details
                        pdv.ClientComments = dr["ClientComments"].ToString();
                        pdv.PDVNote = dr["PDVNote"].ToString();
                    }
                }
            }
        }

        return pdv;
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Dude, you need to talk to your DBA about normalizing that table. \$\endgroup\$ – RubberDuck Jun 5 '15 at 21:17
  • \$\begingroup\$ @RubberDuck I'm also the DBA, its a system that I have inherited and does requires some more normalisation but that's kind of irrelevant for this question as this is a Stored Procedure. \$\endgroup\$ – JammoD Jun 6 '15 at 7:32
  • \$\begingroup\$ It's not irrelevant at all. This looks horrible simply because your model is huge. I'm counting 13 distinct models shoved inside of that one you've got there. \$\endgroup\$ – RubberDuck Jun 6 '15 at 9:42
  • \$\begingroup\$ @RubberDuck I am intrigued how you can tell the database structure from that one model as I am not using Direct Table access with this method and have a stored procedure which is using many tables. I have then created a model based on that stored procedure. How would you proceed? \$\endgroup\$ – JammoD Jun 6 '15 at 9:56
  • 1
    \$\begingroup\$ @RubberDuck Would that not be overkill if each CaseID can only have one set of information? As each case is a property and that property can only have one set of details. The business requirement for all the data is to be loaded into a screen to allow the staff to update and view the information for the property. \$\endgroup\$ – JammoD Jun 6 '15 at 10:11
1
\$\begingroup\$

During our discussion in the comments, it became clear to me that you're mixing the business objects with how they're displayed. How they're displayed has little to do with how you model your business objects. Or, at least it should. You hear the word "model" tossed around a lot, but in reality, there are two different kinds of models. There are business models, and view models. Many times these are identical, sometimes they're not. In this case, I think it's clear that the business model and the view model are almost, but not quite, the same.

Right now, the business wants all of this data in what amounts to a spreadsheet, but what happens when they see the beta version and decide they were wrong and would actually like separate detail screens after all? Then you would have no business or view models to represent these subsets of data. Note that I do not believe this to be a YAGNI situation, because it's very likely that this could happen. If not at beta time, perhaps 6 months from now. Either way, it's likely that you'll need to access these business objects on their own at some point in the future. Set yourself up for success. Minimally, this work will help you to clarify your code.

I imagine your PDV class looking something like this.

public class PDV
{
    DateTime TimeStamp {get; set;}
    PropertyInfo PropertyDetails {get; set;}
    VunerabilityInfo VunerabilityDetails {get; set;}
    ContactInfo Contact {get; set;}
    PaymentInfo PaymentDetails {get; set;}

    //... so and so on
    // basically one for each of the comments in `LoadPDV`
}

Now, you still actually load all of this from the same view, and still all at once (although, now you have the option of going to the database for any one of these objects!). Now, you just create each of these objects from the view data and add it to the PDV.

using (SqlDataReader dr = cmd.ExecuteReader())
{
    while (dr.Read())
    {
        pdv.Timestamp = dr["PDVStamp"];

        var property = new PropertyInfo()
        {
            property.CustContactTel = dr["CustContactTel"].ToString();
            property.CustMobileTel = dr["CustMobileTel"].ToString();
            property.PropertyType = dr["PropertyType"].ToString();
            property.PremisesStatus = dr["PremisesStatus"].ToString();
            property.LockType = string.IsNullOrEmpty(dr["LockType"].ToString()) ? 0 : Convert.ToInt16(dr["LockType"]);
            property.AccessInfo = string.IsNullOrEmpty(dr["AccessInfo"].ToString()) ? 0 : Convert.ToInt16(dr["AccessInfo"]);
        }

        pdv.PropertyDetails = property;

But that's more code! Yes. It is, but we're not going to leave it this way. We're going to extract methods that take in the data reader and return new business objects.

using (SqlDataReader dr = cmd.ExecuteReader())
{
    while (dr.Read())
    {
        pdv.Timestamp = dr["PDVStamp"];
        pdv.PropertyDetails = CreatePropertyDetails(dr);
        pdv.VunerabilityDetails = CreateVunerabilityDetails(dr);

        //...

Now, there's a trade off to be had here. We now have more sensible objects and code. You have the ability to quickly create new views based on these other business objects and the ability to quickly create different ways of getting them back from the database. That's good. The bad part of this is that you'll have to explicitly bind each field to a column in the DataGridView. You know what though, that's not really all that bad. Explicit binding is better in my opinion anyway. Yeah, it's nice to let Visual Studio automatically bind an object to a gridview, but I've been bitten more than once by changing a class's interface, only to have one of my forms suddenly "broken" and not displaying information the same way. Had I explicitly bound it to begin with, it wouldn't have been an issue when I changed the class.

One last thing...

The method GetCommandParameters is used for saving the data

A method that starts with Get should never be saving anything. No side effects. If I call GetFoo, I expect it to retrieve me a Foo, not save that foo to the database.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the answer I will most certainly look at amending my code. I appreciate the time you have taken out to help!! \$\endgroup\$ – JammoD Jun 6 '15 at 11:43
  • \$\begingroup\$ No problem! Have you considered using some Linq-to-sql? I wanted to mention it, but forgot. That could also potentially help clean this up a bit. \$\endgroup\$ – RubberDuck Jun 6 '15 at 11:45
  • \$\begingroup\$ No, I have never really attempted to use any type of Linq code within the application. \$\endgroup\$ – JammoD Jun 6 '15 at 11:46

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.