Skip to content

Conversation

Alexbits
Copy link
Contributor

@Alexbits Alexbits commented Mar 22, 2018

It's better to remove private property setters to make Address value object truly immutable. Otherwise, it's possible to change the object by setting properties inside its methods for example.

protected override IEnumerable<object> GetAtomicValues()
    {
       Street = "Blah blah blah";
       City = "Blah blah blah";

        // Using a yield return statement to return each element one at a time
        yield return Street;
        yield return City;
        yield return State;
        yield return Country;
        yield return ZipCode;
    }

Summary

Remove private property setters. Make public properties readonly.

Fixes #Issue_Number (if available)

It's better to remove private property setters to make Address value object truly immutable. Otherwise, it's possible to change the object by setting properties inside its methods for example.

 protected override IEnumerable<object> GetAtomicValues()
    {
      Street = "Blah blah blah";
      City = "Blah blah blah";

        // Using a yield return statement to return each element one at a time
        yield return Street;
        yield return City;
        yield return State;
        yield return Country;
        yield return ZipCode;
    }
Copy link
Contributor

@jzeferino jzeferino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this @Alexbits We appreciate it.

I've reviewed the changes, and I'm going to :shipit: now.

Pinging @CESARDELATORRE as these changes should make it to the reference application as well.

@BillWagner BillWagner added this to the Sprint 133 (3/17/18 - 4/06/18) milestone Mar 23, 2018
@BillWagner BillWagner merged commit 139ba7d into dotnet:master Mar 23, 2018
@Alexbits Alexbits deleted the patch-1 branch March 23, 2018 15:30
Alexbits referenced this pull request in Alexbits/eShopOnContainers Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants