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

  • Thread starter Thread starter Stefano Peduzzi
  • Start date Start date
S

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
}
}
 
"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
 
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
 
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
 
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
 
"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
 
Back
Top