1

I have a function that creates a new object by passing to it's constructor integer ids primarily with values of 0. This function is called when saving a newly created record (not an edit).

enter image description here

public class RecordController
{
    private int _recordId;
    private int _subRecordId;
    private string _xmlData;

    private SubRecordA CreateSubRecordA()
    {
        _xmlData = LoadXmlData();
        ParseXmlData();

        return new SubRecordA(
            SubRecordTypeId.A  // leaky DAL abstraction
            0,
            0,
            _xmlData);
    }
}

However, the two 0 parameters coincide with private fields that are already initialized to 0 in an Init() method

private void Init()
{
    _recordId = 0;
    _subRecordId = 0;
}

I'm not sure if it is better to explicitly pass these zero-valued fields to SubRecordA's constructor instead?

        return new SubRecordA(
            SubRecordTypeId.A  // leaky DAL abstraction
            _recordId,
            _subRecordId,
            _xmlData);

Passing the literal 0 seems more clear as to what the intention is, but also seems to weaken the logic by excluding these fields from it's flow (say for example _recordId gets initialized with a -1 at some point in the future).


Update

Using Robert Harvey's suggestion to let the default constructor zero out the properties/fields, I created an explicit constructor that takes only the required parameters and calls : this()

public class SubRecordA : SubRecord
{
    public override int SubRecordTypeId { get {return (int)SubRecordTypeId.A; } }
    public int ParentRecordId { get; set; }
    public int RecordId { get; set; }
    public string XmlData { get; set; }

    public SubRecordA(string xmlData) : this()
    {
        //SubRecordTypeId = SubRecordTypeId.A;
        XmlData = xmlData;
    }
}
  • Are the zeros you are passing merely there to insure that recordID and subRecordID have some meaningful initial values? If that's the case, I'd say don't pass them at all. Just set them locally on construction. – Robert Harvey Apr 4 '18 at 15:52
  • Would you ever want to create a SubRecordA with nonzero values in those fields? – 1201ProgramAlarm Apr 4 '18 at 15:53
  • @RobertHarvey Yes, b/c they are actually substituted with temporary (local), negative values that are used as place holders until the records are synced. If there is an exception, the values (0) are restored anyway (it's kind of overkill, but I strive to maintain a consistent state as much as possible). – samis Apr 4 '18 at 15:54
  • What is the meaning of the negative value, and how is that different from zero? Do you do anything special in code with these negative values, like making decisions about program flow? – Robert Harvey Apr 4 '18 at 15:55
  • 2
    OK, well I'm not convinced. The zeros can be automatically assigned at construction, so they don't need to be passed in. And unless the negative values have some special meaning, you don't need them. – Robert Harvey Apr 4 '18 at 15:59
1

Your underlying problem is that 0 is an incorrect value for the id's

Ideally you would change them to GUIDs but a second best would be to change them to nullable uints.

This would allow you to have them set to null untill the object had been persisted and got actual ids

  • I ended up moving the construction of the sub-record to the layer below, where it is persisted along with it's parent-record and all the constructor's parameters are known. The sub-record's xmlData is instead passed along with it's parent-record. – samis Apr 4 '18 at 19:09
  • Because the id's are primary keys, they need to be non-null and unique. The objects get serialzed into a local Sqlite DB that eventually gets synchronized to a remote SQL Server DB, so null/0 isn't an option. They appear in the OP's constructor, but are immediately replaced with temporary (negative) values when the object is serailzed during a call to a persistence layer. I refactored around this with the aforementioned solution above. – samis Apr 4 '18 at 19:18
  • 0 being an incorrect id value is a good point, and your proposed strategy does sound like an effective solution in dealing with that scenario. I am using guids in an auxiliary table of my own, but the main record tables are legacy and need updating. – samis Apr 4 '18 at 19:22

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.