5

My class inherits an interface so I need to have the emailaddress property in the Person class.

My question is what is the best way to get the property and set the property

public class Contact : IPerson, ILocation
{
  ...
  [MaxLength(256)]
  public string EmailAddress { 
  get{
    return this.Emails.First().ToString();
  } 
  set{ ????  }
  }
  ....

  public virtual ICollection<Emails> Emails { get; set; }
}

Essentially, I'm trying to get the class to allow more than one email.

For full disclosure, I'm pretty new to this and I may not be asking the correct questions, but I've searched for a day and half and haven't seen anything like this (not that I think it's unusual) and could use the insight.

Email class properties:

[Key]
public int Id { get; set; }

[MaxLength(256)]
    public string EmailAddress { get; set; }
utd1878
  • 95
  • 1
  • 9
  • if you want to access different indices in your collection, a getter and setter will not be enough. You will have to write a method with some kind of index as parameter – cppanda Dec 31 '12 at 04:03
  • 1
    Fundamentally it *is* unusual to have a collection, but also have a property which tries to handle just a *single* value. How do you want it to behave? If I set the `EmailAddress` property multiple times, should that end up with a single address, or multiple? How would you expect that to interact with setting the `Emails` property? The actual implementation will probably be simple - but you need to work out what behaviour you want first. – Jon Skeet Dec 31 '12 at 04:04
  • I have a one to many between Contacts and their emails (hence the collection). Because I'm inheriting an interface, I can't exclude the EmailAddress property (from the interface). So I'm not sure where to take it from here. Can I ignore the set and just do it through the relationship? – utd1878 Dec 31 '12 at 04:19
  • You can't "ignore" the `set` if it's being forced by the implementation of an interface.. the best you could do is make it throw a `NotImplementedException` in the `set` body. It needs to be there though.. – Simon Whitehead Dec 31 '12 at 04:24

4 Answers4

1

Do you have control over the design of the IPerson interface that is forcing you to implement EmailAddress? If so, I would recommend rethinking the design to avoid needing both a singular property and a list of email addresses on the same object.

You may also want to consider making the Emails property setter protected, to prevent outside code from changing your object's data.

If you must implement to this interface, and you wish to have the EmailAddress property always refer to the first email in the collection, you can try this code.

public class Contact : IPerson, ILocation
{
  public Contact()
  {
    // Initialize the emails list
    this.Emails = new List<Emails>();
  }

  [MaxLength(256)]
  public string EmailAddress
  { 
    get
    {
      // You'll need to decide what to return if the first email has not been set.
      // Replace string.Empty with the appropriate value.
      return this.Emails.Count == 0 ? string.Empty : this.Emails[0].ToString();
    } 
    set
    {
      if (this.Emails.Count == 0)
      {
        this.Emails.Add(new Emails());
      }
      this.Emails[0].EmailAddress = value;
    }
  }

  public virtual IList<Emails> Emails { get; set; }
}
Jeromy Irvine
  • 11,614
  • 3
  • 39
  • 52
  • This seems to be getting me closer. However, it's not liking the .Add() method (saying it's looking for the Email type, not the string type that is value. Also, and I'm not sure what I'm missing, but the Length isn't working on this.Emails. Thanks by the way. UPDATE: changed length to Count :/ – utd1878 Dec 31 '12 at 04:33
  • `Length` is exposed by Array. Other collection exposes `Count` – Tilak Dec 31 '12 at 04:35
  • Oops, good catch on the `Length` thing. The `Add()` will need to be updated per my comment to create a new `Emails` object. Since you didn't include that in the code, I'm not sure what it looks like, so I can't tell you anything more specific. – Jeromy Irvine Dec 31 '12 at 04:40
  • It's a very basic class that has an EmailAddress property. It's sole purpose is just to hold the emails for each contact. Updated the original to show the email class. – utd1878 Dec 31 '12 at 04:43
  • OK, I've updated the code to reflect that. Hope this works out for you. – Jeromy Irvine Dec 31 '12 at 04:50
  • @JeromyIrvine Thanks for the code. It's compiling w/out errors so I will take a look at the rest of the app. As for the .Add(new Emails()), what exactly is that doing? Is that adding the value? – utd1878 Dec 31 '12 at 04:59
  • `.Add(new Emails())` is just putting a new `Emails` object into the list. The `this.Emails[0].EmailAddress = value` line actually sets the value to whatever was passed into the setter. I put it outside of the `if` block to avoid repeating the assignment of the `Emails.EmailAddress` property. – Jeromy Irvine Dec 31 '12 at 05:03
  • So if there isn't an initial email, it puts the new email object, but when does it put the value in? I can see that it would put it in if there are already emails in, but not seeing what happens on the initial insert. Again, I'm fairly new to this, so I'm sorry for asking what are most likely elementary questions. – utd1878 Dec 31 '12 at 05:07
  • @JeromyIrvine Saw your update. Sorry, should have caught that myself. Thanks again for your help and quick reply. – utd1878 Dec 31 '12 at 05:14
  • @JeromyIrvine so one more question, it's now setting both the Contact Class EmailAddress and the Emails class is getting the same value, essentially giving the contact two of the same email addresses. How do I control this? – utd1878 Jan 02 '13 at 03:41
  • In this code, the EmailAddress property is just a shortcut to the first item in the Emails list, so if you look at both properties, it will appear to be duplicated. I would just use the Emails list, if you wish to avoid that. – Jeromy Irvine Jan 02 '13 at 14:45
0

EDIT: I didn't understand the question at first.

In this case, I'm not sure what you are trying to do makes sense. You are taking the first email out of a collection of emails on the getter; so what behavior do you want for the setter? It could (a) update or insert the set value into the first position in the collection or (b) just simply add the email to the collection when set. I'm not sure I would allow setting of that at all, just make a get and leave it at that.

I would also rename the property FirstEmail or something so it is obvious it is coming out of a collection of possible emails.

  public string FirstEmailAddress { 
      get{
         return this.Emails.Count > 0 ? this.Emails.First().ToString() : string.Empty;
      } 
  }
theMayer
  • 15,456
  • 7
  • 58
  • 90
0

Usually, when you want to "show" an internal collection to the world, what you do is make it a property and create only a getter. This way you can not set the entire collection like Contact.Emails = new ICollection<Email>(), which is a bad practice. But you can access the collection through Contacts.Emails and iterate through it or add items, etc.

You can also add an indexer to this property if you like: http://msdn.microsoft.com/en-us/library/vstudio/6x16t2tx(v=vs.100).aspx

Elad Lachmi
  • 10,406
  • 13
  • 71
  • 133
0

From your comment,

I have a one to many between Contacts and their emails (hence the collection). Because I'm inheriting an interface, I can't exclude the EmailAddress property (from the interface). So I'm not sure where to take it from here. Can I ignore the set and just do it through the relationship?

You should not allow public setter. Make it protected as you have marked virtual.

public virtual ICollection<Emails> Emails { get; protected set; }

For your EmailAddress setter you can use following

[MaxLength(256)]
public string EmailAddress 
{ 
    get
    {
        return this.Emails.DefaultIfEmpty(new EMail()).First().EmailAddress;
    } 
    set
    {
        if (Emails == null)
        {
            Emails = new Collection<Email>(); 
        }
        Emails.Items.Insert(0, new EMail {EmailAddress = value}); 
    }
}
Tilak
  • 30,108
  • 19
  • 83
  • 131
  • Doesn't like the new Collection declaration. Or the Items – utd1878 Dec 31 '12 at 05:03
  • but you have exposed like this. It should be exposed as `IList`. Then `Items` will go away. You cannot avoid `Insert` (until it is doubly linked list or Stack) – Tilak Dec 31 '12 at 05:05
  • Sorry, meant the compiler doesn't :) – utd1878 Dec 31 '12 at 05:09
  • I agree about making the `Emails` property protected set, but I don't like the implementation of EmailAddress here. This will throw an error if the getter is called without Emails being initialized, and the use of Insert in the setter will add a new item to the list every time the setter is called, which I do not think is very desirable. – Jeromy Irvine Dec 31 '12 at 05:10
  • @JeromyIrvine, otherwise first email address will always be overwritten. If intention is to overwrite, Emails[0] can be used . `Emails` should be initialized in `ctor` and the condition if (Emails == null) should be removed – Tilak Dec 31 '12 at 05:12
  • @Tilak, I understand, and your code is correct if insertion is desired. I just think it would be cleaner not to have a setter doing insertion. – Jeromy Irvine Dec 31 '12 at 05:19