C# data class with public auto-implemented properties with constructor that specifies each

I have a dll and an executable which both deal in configuring certain things for a much broader software solution, and the code below in particular is directly meant for database configuration. (Here “inmate info” has been substituted for the actual configurations.)

In an MVC context, the dll is essentially the controller, and the executable is effectively the view. There is a data class – Inmate – which represents a central segment of the way data is configured in the database, and it is passed between the dll and the executable. There is a Windows forms class in the executable – InmateInfoForm – which inherits UserControl and is there for the user to specify how the database is configured in this regard.

Consider the following code in InmateInfoForm.cs:

private Inmate m_inmte;
internal Inmate Inmate
{
    get
    {
        if (m_inmte == null)
        {
            m_inmte = new Inmate((int)NumericUpDownCriminalHistoryNumber.Value,
                    (int)NumericUpDownDownIntegrationID.Value,
                    m_eSecurityLevels[ComboBoxSecurityLevel.SelectedIndex],
                    RadioButtonIsInIsolation.Checked,
                    m_eDisciplinaryStatuses[ComboBoxDisciplinaryStatus.SelectedIndex],
                    (int)NumericUpDownAge.Value, TextBoxFirstName.Text,
                    TextBoxMiddleName.Text, TextBoxLastName.Text);
        }
        else
        {
            m_inmte.CriminalHistoryNumber = (int)NumericUpDownCriminalHistoryNumber.Value;
            m_inmte.IntegrationID = (int)NumericUpDownIntegrationID.Value;
            m_inmte.SecurityLevel = m_eSecurityLevels[ComboBoxSecurityLevel.SelectedIndex];
            m_inmte.IsInIsolation = RadioButtonIsInIsolation.Checked;
            m_inmte.DisciplinaryStatus =
                    m_eDisciplinaryStatuses[ComboBoxDisciplinaryStatus.SelectedIndex];
            m_inmte.Age = (int)NumericUpDownAge.Value;
            m_inmte.FirstName = TextBoxFirstNameFirstName.Text;
            m_inmte.MiddleName = TextBoxMiddleName.Text;
            m_inmte.LastName = TextBoxLastName.Text;
        }
        return m_inmte;
    }

and the following code in Inmate.cs:

public int? CriminalHistoryNumber { get; set; }
public int? IntegrationID { get; set; }
public SecurityLevel? SecurityLevel { get; set; }
public bool? IsInIsolation { get; set; }
public DisciplinaryStatus? DisciplinaryStatus { get; set; }
public int? Age { get; set; }
public string FirstName { get; set; }
public string MiddleName { get; set; }
public string LastName { get; set; }

public InmateInfo(int? pCriminalHistoryNumber, int? pIntegrationID, SecurityLevel?
        pSecurityLevel, bool? pIsInIsolation, DisciplinaryStatus? pDisciplinaryStatus, int?
        pAge, string pFirstName, string pMiddleName, string pLastName)
{
    CriminalHistoryNumber = pCriminalHistoryNumber;
    IntegrationID = pIntegrationID;
    SecurityLevel = pSecurityLevel;
    IsInIsolation = pIsInIsolation;
    DisciplinaryStatus = pDisciplinaryStatus;
    Age = pAge;
    FirstName = pFirstName;
    MiddleName = pMiddleName;
    LastName = pLastName;
}

public static Inmate CreateDefaultData(EnumType3 pEnumType3Value, EnumType4
        pEnumType4Value)
{
    // some code
}

In the getter in InmateInfoForm, when m_inmte is equal to null, is it generally better form to leave the code as-is, to go ahead and create a default constructor and use it like this:

        if (m_inmte == null)
        {
            m_inmte = new Inmate();
        }
        else
        {
            m_inmte.CriminalHistoryNumber = (int)NumericUpDownCriminalHistoryNumber.Value;
            m_inmte.IntegrationID = (int)NumericUpDownDownIntegrationID.Value;

The first form shaves off an unnecessary bit of interface and is slightly more direct/efficient, but the second form helps prevent having to put things like (int)NumericUpDownCriminalHistoryNumber.Value (something trivial) in two different places, and it also looks a little bit neater. I included the code about the factory method in Inmate just to say that it’s there, but it’s not particularly useful in the getter inside of InmateInfoForm‘s case. Also the current constructor (which specifies all the properties) is in use inside of some other code that does not run into the scenario above.

Which is generally better form to use and why?

---

Notes:

The Inmate class is being gotten from the UI class as a means of showing the user’s input and keeping it all in one place, so it can be passed around as one reference. (Is the way in which this is being implemented bad?)

Some of the code here is not the exact code in use and lacks some of the context of outside code (e.g., for using nullable types). The UI prefixes would be abbreviated in production (e.g. NumericUpDownSomething => nudSomething).

The structure of the code above is quite close to what’s actually there.

UPDATE

This answer to a more recent question on Programmers.SE is somewhat linked.

You have a constructor with nine parameters, all of them nullable. That’s just asking for problems. Perhaps named arguments could be a solution, but in the context of your code I don’t even see the point. Why not keep it simple, and do this:

    if (m_inmte == null)
    {
        m_inmte = new Inmate();
    }

    m_inmte.CriminalHistoryNumber = (int)NumericUpDownCriminalHistoryNumber.Value;
    m_inmte.IntegrationID = (int)NumericUpDownIntegrationID.Value;
    m_inmte.SecurityLevel = m_eSecurityLevels[ComboBoxSecurityLevel.SelectedIndex];
    m_inmte.IsInIsolation = RadioButtonIsInIsolation.Checked;
    m_inmte.DisciplinaryStatus =
            m_eDisciplinaryStatuses[ComboBoxDisciplinaryStatus.SelectedIndex];
    m_inmte.Age = (int)NumericUpDownAge.Value;
    m_inmte.FirstName = TextBoxFirstNameFirstName.Text;
    m_inmte.MiddleName = TextBoxMiddleName.Text;
    m_inmte.LastName = TextBoxLastName.Text;

Unlike your current code you now only need to adapt one place if there is an extra parameter.

---

I’m worried by the fact that this Inmate is created in a get. At the very least I’d move that to a separate method, but quite frankly it looks like your UI and business logic is mixed together, which is another bad idea.

---

What is string?? A string is already nullable in C#, and I doubt string? is even legit.

---

There are some odd naming conventions in your code, like the prefixes m_ and p. Don’t do that, it makes code so much harder to read.

Also avoid needless abbreviations like inmte.

Don’t prefix form fields with their types, e.g. RadioButtonIsInIsolation or TextBoxFirstNameFirstName.

---

You don’t check your input, e.g. m_inmte.CriminalHistoryNumber = (int)NumericUpDownCriminalHistoryNumber.Value;. What if an invalid value is entered into that text field?