views:

513

answers:

8

Hi everyone,

I am new to programming so i figured i'd get help from those that know it.

I am currently writing a Registration Application which will basically take a users input, validates the data entered, displays a review screen (which the user must print out and mail in a copy), and then save the info entered to a database.

Here are the fields that need to be captured and saved.

public class Person 
{
   private string id;
   private string currentLastName;
   private string currentFirstName;
   private string currentMiddleName;
   private string currentSuffixtName;

   private string formerLastName;
   private string formerFirstName;
   private string formerMiddleName;
   private string formerSuffixtName;

   private string currentAddressNumber;
   private string currentAddressDirection;
   private string currentAddressStreet;
   private string currentAddressStreetType;
   private string currentAddressAptNum;
   private string currentAddressrCity;
   private string currentAddressState;
   private string currentAddressZipcode;
   private string currentAddressCounty;

   private string formerAddressNumber;
   private string formerAddressDirection;
   private string formerAddressStreet;
   private string formerAddressStreetType;
   private string formerAddressAptNum;
   private string formerAddressrCity;
   private string formerAddressState;
   private string formerAddressZipcode;
   private string formerAddressCounty;

   private string mailAddressLineOne;
   private string mailAddressLineTwo;
   private string mailAddressLineThree;

   private DateTime birthdate;
   private string gender;
    private string HomePhone;
    private string WorkPhone;
    private string CellPhone;
    private string FaxNumber;
   private string driversLicense;
   private string ssNumber;
   private string membershipType;
   private DateTime registrationDate;
   private string ipAddress;
   private string browserInfo; 
}

After giving this a another glance I figured i'd seperate the common stuff out and create a class for each of them and have a Person own each of the classes or an Interface for the classes. i.e.

public interface IName
{
    string getLastName();
    string getFirstName();
    string getMiddleName();
    string getSuffixName();
}

    public class Name : IName
{
    private string _lastName;
    private string _firstName;
    private string _middleName;
    private string _suffixName;

    // Validation Methods
    // private set methods

    //#region IName Members
}


public interface IAddress
{
    string getAddressNumber();
    string getAddressDirection();
    string getAddressStreet();
    string getAddressStreetType();
    string getAddressAptNum();
    string getAddressCity();
    string getAddressState();
    string getAddressZipcode();
    string getAddressCounty();
}

public class Address : IAddress
{
    private string _addressNumber;
    private string _addressDirection;
    private string _addressStreet;
    private string _addressStreetType;
    private string _addressAptNum;
    private string _addressrCity;
    private string _addressState;
    private string _addressZipcode;
    private string _addressCounty;

    // Validation Methods
    // private Set Methods
    // public get methods


    //#region IAddress Members
}
public interface IPerson
{
    int getId();
    IName getCurrentName();
    IName getFormerName();

    IAddress getCurrentAddress();
    IAddress getFormerAddress();
    IAddress getMailingAddress();

    DateTime getBirthdate();
    string getGender();
    string getSSNumber();
    string getPersonType();
    DateTime getRegistrationDate();
    string getIPAddress();
    string getBrowserInfo();
    string getDriversLicense();

    string getHomePhone();
    string getWorkPhone();
    string getCellPhone();
    string getFaxNumber();
    string getEmailAddress();
    string getSecondaryEmailAddress();

    bool save();
    void load();

}

public class Person : IPerson
{
    private int _id;
    private IName _currentName;
    private IName _formerName;

    private IAddress _currentAddress;
    private IAddress _formerAddress;
    private IAddress _mailingAddress;

    private DateTime _birthdate;
    private string _gender;
    private string _ssNumber;
    private string _personType;
    private DateTime _registrationDate;
    private string _ipAddress;
    private string _browserInfo;
    private string _driversLicense;

    private string _homePhone;
    private string _workPhone;
    private string _cellPhone;
    private string _faxNumber;
    private string _emailAddress;
    private string _secondaryEmailAddress;


    // private set methods

    // #region IPerson Members
    // .... Get Methods 
    public bool save()
    {
        DataLayer dl = new DataLayer();
        if (_id == 0)
            return dl.insertPerson(this);
        else
            return dl.updatePerson(this);
    }
}

Here's my datalayer insert method

public bool insertPerson(IPerson person)
{
    bool inserted = false;
    SqlConnection cnDB = DatabaseConnection.GetOpenDBConnection();
    try
    {
        SqlCommand cmDB = new SqlCommand("sp_InsertName", cnDB);
        cmDB.CommandType = CommandType.StoredProcedure;
        cmDB.Parameters.Add("@last_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@last_name"].Value = person.getCurrentName().getLastName();
        cmDB.Parameters.Add("@first_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@first_name"].Value = person.getCurrentName().getFirstName();
        cmDB.Parameters.Add("@middle_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@middle_name"].Value = person.getCurrentName().getMiddleName();
        cmDB.Parameters.Add("@suffix_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@suffix_name"].Value = person.getCurrentName().getSuffixName();
        int id = cmDB.ExecuteNonQuery();

        cmDB = new SqlCommand("sp_InsertName", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@last_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@last_name"].Value = person.getFormerName().getLastName();
        cmDB.Parameters.Add("@former_first_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@former_first_name"].Value = person.getFormerName().getFirstName();
        cmDB.Parameters.Add("@middle_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@middle_name"].Value = person.getFormerName().getMiddleName();
        cmDB.Parameters.Add("@suffix_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@suffix_name"].Value = person.getFormerName().getSuffixName();
        cmDB.ExecuteNonQuery();

        // Insert Current Address

        cmDB = new SqlCommand("sp_InsertAddress", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@address_number", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_number"].Value = person.getCurrentAddress().getAddressNumber();
        cmDB.Parameters.Add("@address_direction", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_direction"].Value = person.getCurrentAddress().getAddressDirection();
        cmDB.Parameters.Add("@address_street", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_street"].Value = person.getCurrentAddress().getAddressStreet();
        cmDB.Parameters.Add("@address_street_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_street_type"].Value = person.getCurrentAddress().getAddressStreetType();
        cmDB.Parameters.Add("@address_apt_number", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_apt_number"].Value = person.getCurrentAddress().getAddressAptNum();
        cmDB.Parameters.Add("@address_city", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_city"].Value = person.getCurrentAddress().getAddressCity();
        cmDB.Parameters.Add("@address_state", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_state"].Value = person.getCurrentAddress().getAddressCity();
        cmDB.Parameters.Add("@address_zipcode", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_zipcode"].Value = person.getCurrentAddress().getAddressZipcode();
        cmDB.Parameters.Add("@address_county", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_county"].Value = person.getCurrentAddress().getAddressCounty();
        cmDB.ExecuteNonQuery();

        // Insert Former Address

        cmDB = new SqlCommand("sp_InsertAddress", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@address_number", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_number"].Value = person.getFormerAddress().getAddressNumber();
        cmDB.Parameters.Add("@address_direction", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_direction"].Value = person.getFormerAddress().getAddressDirection();
        cmDB.Parameters.Add("@address_street", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_street"].Value = person.getFormerAddress().getAddressStreet();
        cmDB.Parameters.Add("@address_street_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_street_type"].Value = person.getFormerAddress().getAddressStreetType();
        cmDB.Parameters.Add("@address_apt_number", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_apt_number"].Value = person.getFormerAddress().getAddressAptNum();
        cmDB.Parameters.Add("@address_city", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_city"].Value = person.getFormerAddress().getAddressCity();
        cmDB.Parameters.Add("@address_state", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_state"].Value = person.getFormerAddress().getAddressCity();
        cmDB.Parameters.Add("@address_zipcode", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_zipcode"].Value = person.getFormerAddress().getAddressZipcode();
        cmDB.Parameters.Add("@address_county", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_county"].Value = person.getFormerAddress().getAddressCounty();
        cmDB.ExecuteNonQuery();

        // Insert Mailing Address

        cmDB = new SqlCommand("sp_InsertAddress", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@address_number", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_number"].Value = person.getMailingAddress().getAddressNumber();
        cmDB.Parameters.Add("@address_direction", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_direction"].Value = person.getMailingAddress().getAddressDirection();
        cmDB.Parameters.Add("@address_street", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_street"].Value = person.getMailingAddress().getAddressStreet();
        cmDB.Parameters.Add("@address_street_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_street_type"].Value = person.getMailingAddress().getAddressStreetType();
        cmDB.Parameters.Add("@address_apt_number", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_apt_number"].Value = person.getMailingAddress().getAddressAptNum();
        cmDB.Parameters.Add("@address_city", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_city"].Value = person.getMailingAddress().getAddressCity();
        cmDB.Parameters.Add("@address_state", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_state"].Value = person.getMailingAddress().getAddressCity();
        cmDB.Parameters.Add("@address_zipcode", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_zipcode"].Value = person.getMailingAddress().getAddressZipcode();
        cmDB.Parameters.Add("@address_county", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@address_county"].Value = person.getMailingAddress().getAddressCounty();
        cmDB.ExecuteNonQuery();

        // insert Personal Info

        cmDB = new SqlCommand("sp_InsertPersonalInfo", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@birthdate", System.Data.SqlDbType.DateTime);
        cmDB.Parameters["@birthdate"].Value = person.getBirthdate();
        cmDB.Parameters.Add("@gender", System.Data.SqlDbType.VarChar, 1);
        cmDB.Parameters["@gender"].Value = person.getGender();
        cmDB.Parameters.Add("@ss_number", System.Data.SqlDbType.VarChar, 20);
        cmDB.Parameters["@ss_number"].Value = person.getSSNumber();
        cmDB.Parameters.Add("@person_type", System.Data.SqlDbType.VarChar, 20);
        cmDB.Parameters["@person_type"].Value = person.getPersonType();
        cmDB.Parameters.Add("@registration_date", System.Data.SqlDbType.DateTime);
        cmDB.Parameters["@registration_date"].Value = person.getRegistrationDate();
        cmDB.Parameters.Add("@ip_address", System.Data.SqlDbType.VarChar, 20);
        cmDB.Parameters["@ip_address"].Value = person.getIPAddress();
        cmDB.Parameters.Add("@browser_info", System.Data.SqlDbType.VarChar, 20);
        cmDB.Parameters["@browser_info"].Value = person.getBrowserInfo();
        cmDB.Parameters.Add("@drivers_license", System.Data.SqlDbType.VarChar, 20);
        cmDB.Parameters["@drivers_license"].Value = person.getDriversLicense();
        cmDB.ExecuteNonQuery();

        //insert email address contact type
        cmDB = new SqlCommand("sp_InsertContactType", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@contact_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact_type"].Value = "Email";
        cmDB.Parameters.Add("@contact", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@gender"].Value = person.getEmailAddress();
        cmDB.ExecuteNonQuery();

        //insert secondary email address contact type
        cmDB = new SqlCommand("sp_InsertContactType", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@contact_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact_type"].Value = "Secondary Email";
        cmDB.Parameters.Add("@contact", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@gender"].Value = person.getSecondaryEmailAddress();
        cmDB.ExecuteNonQuery();

        //insert home phone contact type
        cmDB = new SqlCommand("sp_InsertContactType", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@contact_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact_type"].Value = "Home Phone";
        cmDB.Parameters.Add("@contact", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact"].Value = person.getHomePhone();
        cmDB.ExecuteNonQuery();

        //insert work phone contact type
        cmDB = new SqlCommand("sp_InsertContactType", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@contact_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact_type"].Value = "Work Phone";
        cmDB.Parameters.Add("@contact", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact"].Value = person.getWorkPhone();
        cmDB.ExecuteNonQuery();

        //insert cell phone contact type
        cmDB = new SqlCommand("sp_InsertContactType", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@contact_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact_type"].Value = "Cell Phone";
        cmDB.Parameters.Add("@contact", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact"].Value = person.getCellPhone();
        cmDB.ExecuteNonQuery();

        //insert cell phone contact type
        cmDB = new SqlCommand("sp_InsertContactType", cnDB);
        cmDB.Parameters.Add("@person_id", System.Data.SqlDbType.Int);
        cmDB.Parameters["@person_id"].Value = id;
        cmDB.Parameters.Add("@contact_type", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact_type"].Value = "Fax Number";
        cmDB.Parameters.Add("@contact", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@contact"].Value = person.getFaxNumber();
        cmDB.ExecuteNonQuery();


        inserted = true;
    }
    catch (SqlException sqlEx)
    {
        throw new Exception(GetSqlExceptionMessage(sqlEx.Number));
    }
    catch (Exception ex)
    {
        throw ex;
    }
    finally
    {
        if (cnDB.State == ConnectionState.Open)
            cnDB.Close();
    }
    return inserted;
}

Edit: I feel this code will be extremely hard to maintain later on. Can anyone help me figure out a more simple/maintainable way to do this? I'm also stumped on how i should go about updating a record after it has been added.

Thanks in advance for any help

+1  A: 

I notice your using ASP.NET, you should know that the Membership Provider can handle all of that grunt work for you.

BBetances
While the built-in provider can help, it won't handle the full scenario - see http://msdn.microsoft.com/en-us/library/aa478949.aspx#data_schema for the info it saves. Granted you can write a custom provider, but then you arrive at the same, how to structure the zSysop scenario.
eglasius
A: 

Thanks for the heads up, but some of these classes might be re-used in a windows c# app or a java app and i don't think they have a membership provider like ASP.NET.

Thanks

yes, but since the database is already created for you, you can query against that. Even still, too many strings and interfaces. If you going to use Java (which you shouldn't), then just pass the entered username and password and authenticate against it.
BBetances
A: 

I'm curious as to how you will handle future updates to a person's address, etc. Instead of merging current and previous elements into one object, you may want to consider an audit trail table. Have your Person object only contain one set of data. Query the actual table for current data, and query against the audit table for previous and "previous-previous" data. Unless, for some odd reason, the software requirements call for previous first name, etc., up front on person creation.

HardCode
The software requirements state that if a person being entered has a former name or a former address it must be captured and displayed upon searches or print jobs.We have a history database capturing changes to the records after the record has been input as well, but thats for audit trail purposes.
zSysop
+2  A: 

Based on the requirements you listed, you don't seem to be needing to do much manipulation of the data in the application. Particularly it looks like some of these can be treated as lists of data associated with the user i.e. lists of names, addresses and phones. The data access code you posted already seems to be structured this way, I don't think in your scenario you have a need to make the logic side of your application more rigid.

Given names are being treated like a list of data related to the person, I don't think you want to make the first insert of it the piece that creates the user. You have other information you can add to the main record, like driversLicense, ssn, birthdate.

For interfaces consider if you are getting any out of it. It is a nice way to break direct dependencies between the classes, but it doesn't look you have that scenario in the code. Also note that declaring interfaces doesn't force you to use get/set method, as you can require a property with a get to be implemented by the class.

Add a type property to each of the classes used in the lists. You can use this to reduce the code in both the interface and the data access layer. In the later, you can now just loop through the list and insert all of those in the same way.

Consider to use something that helps you with the data access, like linq2sql or nhibernate. This will simplify the data access code and make it less error prone.

Ps. there is not a single solution for all scenarios, and even in specific scenarios personal experiences will affect the way each person tackles the problem. There are practices and principles meant to reduce this, but even then differences do appear. That said, I suggest you read some info on SOLID, DDD and TDD. They are not a recipe for these problems, but learning about those definitely helps software development.

Update: About questions in the comments. Linq2sql supports stored procedures, check scott gu's series on linq2sql: http://weblogs.asp.net/scottgu/archive/2007/09/07/linq-to-sql-part-9-using-a-custom-linq-expression-with-the-lt-asp-linqdatasource-gt-control.aspx. You loose part of the flexibility of linq2sql by using sp, but it will help you anyway.

Regarding the updates. There are different strategies, the main issue is regarding concurrent updates to the same info. If concurrent updates is not an issue, then you can just leave the latest version, so you just save the data. In that case the only original data you need is the ID of the record, and you let the data be overwritten. If you need to detect concurrent updates, you have to either keep a copy of all the original values or use a timestamp. The sp code needs to check original values match when updating the record.

You can configure linq2sql in the designer to control what type of concurrency update strategy you are using and what original values you are sending. There are probably some questions/answers in SO related to this. How you specifically hook it up, will depend on the use you are giving it (using its generated entities or rolling your own, attaching sp to the entities or separately) and even the controls you use, as some of them can hold original values for you (using viewstate).

eglasius
Thanks for the help. I'll move addresses/contact types/names into a list.Does linq2Sql work with stored procedures? Cause all i can use to access the data is stored procedures.These records will be searchable/updatable as well. Do you have any tips on how i should go about tackling this?
zSysop
@zSysop added an update, I suggest you look at the linq2sql series, and consider how it fits in what you want to achieve. Do some work on it, and search info on more specific questions that may appear (of course, if there isn't info on it you can post further questions).
eglasius
Upvoted because of the comment about interfaces... we need more developers that will think critically and evaluate a "best practice" for it's real benefit to a specific situation... this situation does not scream using interfaces for things like Name, Address and Person (at least to me). YAGNI.
Mike Stone
A: 

Sorry, I'm not going to have a complete solution here for you (just wanted to start with that).

I'd really recommend using some kind of generated Data Access Layer; eg. Linq to SQL or NHibernate (there are more, but those two are both pretty popular). The code you are saying you will be difficult to maintain is code you really shouldn't have to maintain. Let somebody else do the grunt work there.

As an aside, I'd say you are doing the right thing with splitting up all of the data into separate classes that implement interfaces. One thing I'll mention though -- with .Net you don't need to make explicit "getxxxx()" methods; Use a property with a public getter. It'll look something like this:

public class Name : IName
{
    private string _lastName;
    private string _firstName;
    private string _middleName;
    private string _suffixName;

    public string LastName { get { return _lastName; } }

    //Alternative automatic property:
    public string FirstName { get; protected set; } 
}
Chris Shaffer
I like your comments, except the part about interfaces... not every class needs to be an interface, and I would agree with Freddy Rios that whether there is any real benefit of breaking them up should be considered.
Mike Stone
I think the interfaces are useful in this case because the "former" versions may actually be a different class (eg, a FormerAddress class), allowing it to operate differently than the Address class but sharing a common interface.
Chris Shaffer
+3  A: 

First if you want to simplify maintaneance, I would look at using an ORM to do the mapping. I'd look at nHibernate, Linq2Sql, Entity Framework or one of the other billion or so out there. It will remove a lot of noise from above.

Second suggestion would be to continue the refactoring you were doing. I like how you absrtacted Address, if you look at your Person, a Person doesn't have a previous name, what your doing is leaking a requirement to track previous information, within your domain model. I would move all of that out, and define a DTO, which would have two properties:

public class PersonDTO { public Person Current; Public Person Previous; }

I think this cleans up a lot of the noise. I also like to handle the multiple phone numbers using a dictionary essentially we have:

public class Phone { PhoneType PhoneType; string Number string AreaCode string Extension }

Some people might make Phone have numerics, so I don't want to debate that, or you could just have a single string like you do. One thing to keep in mind is Internaitonalization if this applies.

Now on my person I would have a Dictionary, this allows you then to break your four inserts into a single loop as you loop over the phone dictionary. I would use a simillar model if you have a billing or ship to address.

If you don't go with an ORM, one way to help adhere to DRY with your sql params is simple. The following code is not meant to be compiled but just to show you a pattern to again minimize noise:

public class DALLayer
{
   public void InsertPerson(Person p)
   {
       using (SqlCommand sqlCmd=CreateNewCommand())
       {
           AddPersonParams(p,sqlCmd);
           sqlCmd.ExecuteNonQuery();
       }
   }

   public void UpdatePerson(Person p)
   {
       using (SqlCommand sqlCmd=CreateNewCommand())
       {
           AddPersonParams(p,sqlCmd);
           //We add the Id since we have it in updates but not in inserts
           sqlCmd.AddParameter("@PersonId",p.Id);
           sqlCmd.ExecuteNonQuery();
       }

   }

   private void AddPersonParams(Person p, Sqlcommand sqlCmd)
   {
       //Do all the code to add Params which exists in both insert and updates
   }

}

Now what you've done is consolidated so when you add a new property to a person you only have to modify one function when dealing with Parameters.

One more thing to insert your current and previous address, you are not being DRY. You could do this:

private void InserAddress(Address address, int Id, AddressType addType)
{

}

Now from your insert Person routine you would call this method twice:

InsertAddress(p.CurrentAddress,id,CurrentAddress);
InsertAddress(p.FormerAddress,id,FormerAddress);

Of course if you take my earlier suggestions this might look very different.

Edit

I totally missed this but I wanted to highlight what Chris said (and I +1 you but felt it was such a good thing I had to repeat it. Use Properties:-)

JoshBerke
Thanks Josh.You've been a real help.I'm new to ORM so it'll take awhile for me to figure out how to use it with stored procedures, but i'll give it a go.Thanks once again!
zSysop
Glad it helped good luck....
JoshBerke
+1 right on - leaked requirement into the domain.
eglasius
+1  A: 

I would have the person record have foreign keys to the sub tables (names, addresses, contact info, etc).

Then I have a plain old object with properties for each field and a repository class that is responsible for mapping the class to the database stored procedure parameters.

I didn't fill in all of the fields but there should be enough here to get you going. Also, you can extract interfaces for each class to program against (for testing, mocking, etc).

Deciding if a save should insert or update would involve writing the udpate statements and then doing

public void Save()
{
    string.IsNullOrEmpty(Id) ? Insert() : Update();
}

Cleaned up data classes:

using System;
public class PersonName
{
    public string Id { get; set; }

    public string Last { get; set; }
    public string First{ get; set; }
    public string Middle{ get; set; }
    public string Suffix { get; set; }
}

public class PersonNameRepository
{
    public void Insert(PersonName pa)
    {
        SqlCommand cmDB = new SqlCommand("sp_InsertName", cnDB);
        cmDB.CommandType = CommandType.StoredProcedure;
        cmDB.Parameters.Add("@last_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@last_name"].Value = pa.Last;
        cmDB.Parameters.Add("@first_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@first_name"].Value = pa.First;
        cmDB.Parameters.Add("@middle_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@middle_name"].Value = pa.Middle;
        cmDB.Parameters.Add("@suffix_name", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@suffix_name"].Value = pa.Suffix;
        int id = cmDB.ExecuteNonQuery();
        pa.Id = id.ToString();
    }
}

public class PersonAddress
{
    public string Id { get; set; }

    public string Number { get; set; }
    public string Direction { get; set; }
    public string Street { get; set; }
    public string City { get; set; }
}

public class PersonAddressRepository
{
    public void Insert(PersonAddress pa)
    {
        // Same as PersonNameRepository
    }
}


public class PersonInfo
{
    public string Id { get; set; }

    public DateTime BirthDate { get; set; }
    public string Gender { get; set; }
    public string SsNumber { get; set; }

}

public class PersonInfoRepository
{
    public void Insert(PersonInfo pa)
    {
        // Same as PersonNameRepository
    }
}
public class Person
{
    public string Id { get; set; }
    public PersonName CurrentName { get; set; }
    public PersonName FormerName { get; set; }

    public PersonAddress CurrentAddress { get; set; }
    public PersonAddress FormerAddress { get; set; }

    public PersonInfo Info { get; set; }
}

public class PersonRepository
{
    public void Insert(Person p)
    {
        // TODO: Begin Transaction

        p.CurrentName.Insert();
        p.FormerName.Insert();
        p.CurrentAddress.Insert();
        p.FormerAddress.Insert();
        p.Info.Insert();

        SqlCommand cmDB = new SqlCommand("sp_InsertPerson", cnDB);
        cmDB.CommandType = CommandType.StoredProcedure;
        cmDB.Parameters.Add("@current_name_id", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@current_name_id"].Value = p.CurrentName.Id;
        cmDB.Parameters.Add("@former_name_id", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@former_name_id"].Value = p.FormerName.Id; ;
        cmDB.Parameters.Add("@current_address_id", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@current_address_id"].Value = p.CurrentAddress.Id;
        cmDB.Parameters.Add("@former_address_id", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@former_address_id"].Value = p.FormerAddress.Id;
        cmDB.Parameters.Add("@info_id", System.Data.SqlDbType.VarChar, 50);
        cmDB.Parameters["@info_id"].Value = p.Info.Id;
        int id = cmDB.ExecuteNonQuery();
        pa.Id = id.ToString();

        // TODO: End Transaction
    }

}
Alan Jackson
A: 

In general there are a lot of good answers that demonstrate distinct Data Access and Business Logic layers. One thing I would add on top of that is getting rid of the Interfaces for every single class. Writing an interface when you know for sure there's only ever going to be one class implementing it is over-design and needlessly complicates things. Now if you were writing a class library and were only offering the one class as a template or starting point for others, then that would be different. But the interfaces shown here didn't exactly leave a lot of room for re-interpretation.

Jeremy