-3

Hello guys, please don't be too harsh. I am a beginner.

What is the best practice for and what is the difference b/n the following:
#1

public SodaDateTime DteRegistered
{    
    get { return DateTimeUtil.NullDateForMaxOrMinDate(this._dteRegistered); }
    set
    {
        if (DateTimeUtil.IsNullDate(value))
        {
            this._dteRegistered = new SodaDateTime("DteRegistered", this, DateTime.Today);
        }
        else
        {
            this._dteRegistered = new SodaDateTime("DteRegistered", this, value);
        }
    }
}

VS.
#2

public SodaDateTime DteRegistered
{
    get
    {
        if (DateTimeUtil.IsNullDate(this._dteRegistered))
        { 
            _dteRegistered = new SodaDateTime("DteRegistered", this, DateTime.Today); 
        }
        return this._dteRegistered;
    }
    set { _dteRegistered = new SodaDateTime("DteRegistered", this, value); }
}
Rufus L
  • 36,127
  • 5
  • 30
  • 43
Niki
  • 1
  • 1
  • 2
    Aside from the fact that the first one calls `NullDateForMaxOrMinDate` and the second one doesn't, one does validation in the setter and the other in the getter. Which one you choose is a matter of opinion (which is why I voted to close), though I prefer doing validation in the setter. That is also where I would call `NullDateForMaxOrMinDate` (before setting the private backing field). – Rufus L Feb 09 '18 at 20:24
  • I concur on closing (Opinion request) and prefering validation in the setter. I would not use a silent override, but an exception as reactiong to a faulty SodaDateTIme. This is an issue "that should not be ignored" or silently fixed. (http://www.codeproject.com/Articles/9538/Exception-Handling-Best-Practices-in-NET) I also wonder about this whole design: SodaDateTime seems to be derived of DateTime and adds 2 fields - 1 string and a reference to the class you assign it too? What is up with that? – Christopher Feb 09 '18 at 20:29

1 Answers1

1

Microsoft published a Property Design Document which provides the following guidance that I believe is applicable to your question:

✓ DO provide sensible default values for all properties, ensuring that the defaults do not result in a security hole or terribly inefficient code.

✓ DO preserve the previous value if a property setter throws an exception.

X AVOID throwing exceptions from property getters. Property getters should be simple operations and should not have any preconditions. If a getter can throw an exception, it should probably be redesigned to be a method.


Based on the guidance that a property should have a valid default value and that getters should be simple operations, I would suggest that you do validation in the setter. It also might be reasonable to throw an ArgumentOutOfRangeException in the setter if the value passed is not valid, so that the client working with your class understands what's happening

// Provide reasonable default value
private SodaDateTime _dteRegistered = 
    new SodaDateTime("DteRegistered", this, DateTime.Today);

// Getter is a simple operation without preconditions
public SodaDateTime DteRegistered
{    
    get { return _dteRegistered; }
    set
    {            
        if (value == null) 
        { 
            throw new ArgumentOutOfRangeException("value cannot be null");
        }

        if (DateTimeUtil.IsNullDate(value)) 
        { 
            throw new ArgumentOutOfRangeException("value cannot have a null date"); 
            // or: value = DateTime.Today;
        }

        this._dteRegistered = DateTimeUtil.NullDateForMaxOrMinDate(
            new SodaDateTime("DteRegistered", this, value));
    }
}
Rufus L
  • 36,127
  • 5
  • 30
  • 43
  • Why ArgumentOutOfRangeException over ArgumentNullException? – Blake Thingstad Feb 09 '18 at 21:26
  • @BlakeThingstad That's a great point. I'm not familiar with the `SodaDateTime` type, but if `value == null` then I'd definitely use `ArgumentNullException`. I updated my answer – Rufus L Feb 09 '18 at 22:11