Instantiating a property when the required attributes have been set

Question

I have a simple class which holds my product. Someone can add a serial to this product and also a purchase id. On change of the properties, I want to check whether both properties are set, and get the license class and add it to the product.

public class License
{
    public bool IsTrial { get; set; }

    public bool IsValid { get; set; }

    public string Type { get; set; }

    public License()
    {

    }

    public License(string purchaseId, string serial)
    {
        this.IsTrial = true;
        this.IsValid = true;
        this.Type = "Enterprise";
    }

Example License class

public class Product
{
    private License _license;
    public License license
    {
        get
        {
            return _license;
        }
        set
        {
            _license = value;
        }
    }

    public string Name { get; set; }

    private string _purchaseId;
    public string PurchaseId {
        get
        {
            return _purchaseId;
        }
        set
        {
            _purchaseId = value;
            GetLicense();
        }
    }

    private string _serial;
    public string Serial
    {
        get
        {
            return _serial;
        }
        set
        {
            _serial = value;
            GetLicense();
        }
    }

    private void GetLicense()
    {
        if (!string.IsNullOrEmpty(this.Serial) && !string.IsNullOrEmpty(this.PurchaseId))
        { 
            License lice = new License(serial, PurchaseId);
            this.License = lice;
        }
    }

That's the approach I used. Is this a correct way to do it? How is this called and what is the best practice? I need to use .NET Framework 3.5.

Answer 1

Getters & Setters + Validation

I think it's better to validate the serial and the purchase-id in their setters instead in the GetLicense method that actually could be a readonly-property.

private void GetLicense()
{
    if (!string.IsNullOrEmpty(this.Serial) && !string.IsNullOrEmpty(this.PurchaseId))
    { 
        License lice = new License(serial, PurchaseId);
        this.License = lice;
    }
}

After this tiny refactorings the result is:

public class Product
{
    // it's prettier if you group private fields togehter at the top
    private string _purchaseId;
    private string _serial;   

    public string Name { get; set; }

    public string PurchaseId
    {
        get { return _purchaseId; }
        set
        {
            if (string.IsNullOrEmpty(value)) 
                throw new ArgumentNullException(nameof(PurchaseId));
            _purchaseId = value;
        }
    }


    public string Serial
    {
        get { return _serial; }
        set
        {
            if (string.IsNullOrEmpty(value)) 
                throw new ArgumentNullException(nameof(Serial));
            _serial = value;
        }
    }   

    public License License
    {
        get { new License(Serial, PurchaseId); }
    }
}

---

Magic Strings

this.Type = "Enterprise";

You should create a const/readonly string for the "Enterprise" and name it for example DefaultLicenseType

---

Bug

There is a bug. The license constructor expects purchaseId + serial and you create a license with a serial + purchase id

Constructor signature:

public License(string purchaseId, string serial)

License creation:

new License(serial, PurchaseId);

Answer 2

The constructor of License takes two arguments that are never used:

public License(string purchaseId, string serial) // makes no sense
{
    this.IsTrial = true;
    this.IsValid = true;
    this.Type = "Enterprise";
}

---

The method private void GetLicense() is pointless, because any check it performs on the state of the Product object (namely the properties of its _license property) can easily circumvented by setting an arbitrary value via the public setter License:

public License license
{
    get
    {
        return _license;
    }
    set
    {
        _license = value; // where value = new License("warez", "crew 1337");
    }
}

Answer 3

Sorry, am late to the party. As it was already pointed, Some properties were not used including the Name property. You are also breaking one of the SOLID principles - Single Responsibility. Each class should be responsible for one action - your product class should not be responsible for getLicense as the License class should be responsible for that.

Validation is the key here as @t3chb0t specified. Although, there may not be any need for the getLicense in this case but be mindful of your naming conventions. In java, you have get and set methods ; set has a return type void in most cases and get has a return type that is not void. This is quite similar to properties in C#(get and set)

 private void GetLicense()
    {
        if (!string.IsNullOrEmpty(this.Serial) && !string.IsNullOrEmpty(this.PurchaseId))
        { 
            License lice = new License(serial, PurchaseId);
            this.License = lice;
        }
    }

should have been

  private void SetLicense()
    {
        if (!string.IsNullOrEmpty(this.Serial) && !string.IsNullOrEmpty(this.PurchaseId))
        { 
            License lice = new License(serial, PurchaseId);
            this.License = lice;
        }
    }

Note: this keyword is synonyms with java, in c# you can use

isTrial = true