Generics for Customer and Phones.... should be easy!

[Stefano Peduzzi]

Hi,
I'm building an application where I've defined a custom class Customer.
Customer can have many phones (defined by phoneType and phoneNumber). I want
to check that a phoneNumber is not already present in customer phones. So
i've build a PhoneCollection class inheriting for List<Phone> and defined an
Add method to check if the phoneNumber already exists. Then I added the
PhoneCollection to Customer properties.
Everything works fine, and I can add new phones to customer using
cust.Phones.Add("1234") but Customer.Phones has Count=0 and if I look at it
in the debugger the phones are inside rawData (??). Strange thing is that if
I use customer.Phones i got the i phone object!
What am I missing?? Below you can find the classes used.
Any help is welcome!
Bye,
Stefano

public class Customer
{
private String id;
private readonly PhoneCollection phones = new PhoneCollection();

public Customer(string id)
{
this.id = id;
}

public PhoneCollection Phones
{
get
{
return this.phones;
}
}
}

public class Phone
{
private string id;
private string ownerId;
private int phoneType;
private string phonenumber;

public Customer(string id)
{
this.id = id;
}

public int PhoneType
{
get{return phoneType;}
set{phoneType = value;}
}

public string PhoneNumber
{
get{return phoneNumber;}
set{phoneNumber = value;}
}
}

public class PhoneCollection: List<Phone>
{
private List<Phone> m_Phones = new List<Phone>();

public PhoneCollection()
{
m_Phones.Capacity = 10;
}

public void Add(string phoneNumber)
{
//Here I add a new Phone to m_Phones if the phoneNumber is not present
}
}

 

[Joanna Carter [TeamB]]

"Stefano Peduzzi" <[email protected]> a écrit dans le message de (e-mail address removed)...

| public class PhoneCollection: List<Phone>
| {
| private List<Phone> m_Phones = new List<Phone>();

If you are inheiting from List<Phone>, then you do not need to also have an
internal list.

| public PhoneCollection()
| {
| m_Phones.Capacity = 10;
| }
|
| public void Add(string phoneNumber)
| {
| //Here I add a new Phone to m_Phones if the phoneNumber is not present
| }
| }

The problem is that you are inheriting from a class and then using an inner
list to store the numbers rather than the instance of the PhoneCollection
class, which is a list in itself. Also, declaring your own Add method will
hide the original Add method and should have given you a warning.

If I were you I would restructure this to declare your own class that no
longer inherits, but that simply contains a list, or better still a
Dictionary<K,V>.

public class PhoneCollection
{
private Dictionary<string, Phone> phones; = new Dictionary<string,
Phone>();

public void Add(string phoneNumber)
{
if (!phones.ContainsKey(phoneNumber))
phones.Add(phoneNumber, new Phone(...));
}

public Phone this[string number]
{
get { return phones[number]; }
}

public Phone this[int index]
{
get { return phones.Values[index]; }
}
}

Joanna

 

[Marc Gravell]

You are mixing inheritance and encapsulation; badly. Either you *are* a
list, or you *contain* the list. In your case, you do both. The methdos
you haven't overridden are pointing at the inherited list, where-as
..Add is looking at the contained list.

Basically, remove m_Phones completely; you *are* the list. Replace the
m_Phones.Something() methods to base.Something() - i.e. call the base
version of this method - i.e.

public void Add(string number) {
if(!Contains(number)) base.Add(number);
}

Although, personally I don't like this usage as people generally expect
..Add to either add or throw.

Marc

 

[Stefano Peduzzi]

Hi Joanna,
Thanks for your help! I tried the "route" you proposed and it is really
interesting (it works with 5 lines of code!). I've found 2 problems with it:
- > public Phone this[int index]{ get { return phones.Values[index]; }}
gives me this compiler error:
Cannot apply indexing with [] to an expression of type
'System.Collections.Generic.Dictionary<string,BusinessEntity.Phone>.ValueCollection'
C:\Progetti\Test\BusinessEntity\PhoneCollection2.cs 28 18 BusinessEntity
-> If I write
foreach (Phone tmp in c.Phones2)
I get this compiler error:
Error 1 foreach statement cannot operate on variables of type
'BusinessEntity.PhoneCollection2' because 'BusinessEntity.PhoneCollection2'
does not contain a public definition for 'GetEnumerator'
C:\Progetti\Test\Test\Form1.cs 97 13 Test

As you can see... if you have some good link on Dictionary it would be
useful!
Thanks,
Stefano

 

[Stefano Peduzzi]

Hi Marc,
As for Joanna... thanks for your help! Also your hints worked, so I removed
m_Phones. I have now a problem with Contains that always returns true. I've
implemented the Phone class inheriting from IComparable<Phone> and I defined
Equals and CompareTo.. I think (...) it is not enough: what should I
implement to have Contains working?

Thanks,
Stefano

 

[Joanna Carter [TeamB]]

"Stefano Peduzzi" <[email protected]> a écrit dans le message de %[email protected]...

| Thanks for your help! I tried the "route" you proposed and it is really
| interesting (it works with 5 lines of code!). I've found 2 problems with
it:
| - > public Phone this[int index]{ get { return phones.Values[index]; }}
| gives me this compiler error:
| Cannot apply indexing with [] to an expression of type
|
'System.Collections.Generic.Dictionary<string,BusinessEntity.Phone>.ValueCollection'

Sorry, my bad, I was mixing List<T> code Do you really need an integer
index ?

| C:\Progetti\Test\BusinessEntity\PhoneCollection2.cs 28 18 BusinessEntity
| -> If I write
| foreach (Phone tmp in c.Phones2)
| I get this compiler error:
| Error 1 foreach statement cannot operate on variables of type
| 'BusinessEntity.PhoneCollection2' because
'BusinessEntity.PhoneCollection2'
| does not contain a public definition for 'GetEnumerator'
| C:\Progetti\Test\Test\Form1.cs 97 13 Test

Then add the IEnumerable<Phone> interface to PhoneCollection and wire it to
the dictionary.

public class PhoneCollection : IEnumerable<Phone>
{
IEnumerator<Phone> GetEnumerator()
{
return phones.Values.GetEnumerator();
}
}

| As you can see... if you have some good link on Dictionary it would be
| useful!

Take a look at the examples in the help.

Joanna