Jump to content
  • Advertisement
Sign in to follow this  
Neith

Adding a new copy of an object to a List

This topic is 1842 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Okay, bear with me while I explain this...

 

I have a class called Item (this isn't truly it, just an example):

public class Item
{
     public string Name;
     public float Weight;

     public Item(string Name)
     {
        this.Name = Name;
     }
}

I also have an Item Handler class, again something similar to this:

public class ObjectHandler
{
   private Dictionary<string, Item> objects = new Dictionary<string, Item>();

   public void Add(Item item)
   {
        if (!objects.ContainsKey(item.Name.ToLower()))
             objects.Add(item.Name, item);
        else objects.Add(item.Name, item);
   }

   public Item Create(string Name)
   {
        foreach (KeyValuePair<string, Item> obj in objects)
        {
             if (Name.ToLower() == obj.Key.ToLower())
                  return obj.Value;
        }
        return null;
   }
}

First off, I go about creating some Items:

Item orange = new Item("orange");
orange.Weight = 1f;
ObjectHandler.Add(orange); // adds orange to object handler's dictionary

I have another class that contains a List<Item>, let's call it Room.

 

 

So here's my problem...I know if I do this:

Room.List.Add(ObjectHandler.Create("orange"));

The original orange Item will be added to the Room as a reference.  Thus, if I change the orange in Room's List, the one in the ObjectHandler's list will change too and if I try to .Create another orange later, that one will contain the changes.  I want to be able to .Create the original one, and only supply a NEW instatiation of orange with all the properties as I originally set them.

 

I know I could just make Orange a class that extends Item, but for these purposes I'd rather not have to make a new class for each hundred items I want to make.

 

Even if I make Item IClonable and do something like this:

public object Clone()
{
     return this.MemberwiseClone();
}

It will keep the changes I set to the Item.  Even if I make a copy constructor for Item, or a method in Item that returns a new Item with fields equal to this., it won't work.  That latter example really confuses me...if it sends back:

// from some CopyMe method in Item
return new Item("")
{
     Name = this.Name,
     //etc.
};

...even then the List is still referencing the changing object.  baaaaa!

 

Please help me understand what's going on, and how to do what I'm trying to do.

Edited by Neith

Share this post


Link to post
Share on other sites
Advertisement

Nevermind...I think it works if, instead of

Room.List.Add(ObjectHandler.Create("orange"));

I do:

Item anotherOrange = ObjectFactory.Create("orange");
anotherOrange.Name = "n00b";
Room.List.Add(anotherOrange);

or even:

Room.List.Add((Item)ObjectHandler.Create("orange"));

Although, if Item contains a List, that List keeps changing, no matter the above "solutions"...and I see why...it's the same referencing problem.

 

Is there no way to make all the members of a class "new" when making the a new instance?  I mean, shouldn't that happen automatically? Or is it just certain types, like List? 

 

For example, the floats and the strings are all fine, but the Lists don't create new copies of themselves.  Which means, in my "return new Item() {}" I have to do something like:

public Item Clone()
{
     Item c = new Item(console);
     c.Name = this.Name;
     //etc.
     c.whateverList = new List<Item>(this.whateverList);

     return c;
}


Which, I suppose if I have to, will work, but I don't get how, will all these "new"s all over the place -- including in the member fields themselves when the Item is instantiated -- not everything's "new".

 

And if I extend Item, I'll have to...I dunno, virtualize Clone() to get the new class's Lists, Dictionaries, etc. It's probably just C#, and I'll have to make due, unless you've managed to read all of this and understand a better way.

 

grrr

Edited by Neith

Share this post


Link to post
Share on other sites

You have a whole bunch of weird stuff going on here.
First, this class.

public class ObjectHandler
{
   private Dictionary<string, Item> objects = new Dictionary<string, Item>();

   public void Add(Item item)
   {
        if (!objects.ContainsKey(item.Name.ToLower()))
             objects.Add(item.Name, item);
        else objects.Add(item.Name, item);
   }

   public Item Create(string Name)
   {
        foreach (KeyValuePair<string, Item> obj in objects)
        {
             if (Name.ToLower() == obj.Key.ToLower())
                  return obj.Value;
        }
        return null;
   }
}

Your "Add" method checks if the key is already there, but it does the same thing regardless?

Your "Create" method doesn't create anything. It's actually just a poor implementation of TryGetValue (if you want to compare names in lower case, add the keys in lower case as you go).
 

The original orange Item will be added to the Room as a reference.  Thus, if I change the orange in Room's List, the one in the ObjectHandler's list will change too and if I try to .Create another orange later, that one will contain the changes.  I want to be able to .Create the original one, and only supply a NEW instatiation of orange with all the properties as I originally set them.
 
I know I could just make Orange a class that extends Item, but for these purposes I'd rather not have to make a new class for each hundred items I want to make.


I'm not sure you understand how objects work. You can create as many Items as you like and they can all have the name "orange" and are all separate distinct objects. The issue is that you are storing the objects in a Dictionary by name. A Dictionary is designed to have one value per key.

 

Can you show the definition of Room (specifically the Room.List member)? 

Share this post


Link to post
Share on other sites

Your "Add" method checks if the key is already there, but it does the same thing regardless?

I know...it was an oversight.  Like I said, those aren't the actual classes I'm working with, just a way for me to explain what I'm doing.  I'm so very sorry.
 
 
 

Your "Create" method doesn't create anything. It's actually just a poor implementation of TryGetValue

 
I know it doesn't actually "create" an object...it's just a name for the idea of what it does: here's the name of a pre-set-up item, get it out of your list of pre-set-up-items, make a copy, and give that copy to me.
It doesn't actually look like the code up there; it actually returns this:

return obj.Value.Clone();

I'm not sure you understand how objects work.

 
Thank you.  Though it's more likely I'm not sure how some things work in C#.
 
 
 

The issue is that you are storing the objects in a Dictionary by name. A Dictionary is designed to have one value per key.

 
And that key is an instantiated Item.  Well, the key's a string, the thing i'm storing is the item. 

 

I'm not trying to add more items to the ObjectHandler...it only needs one "orange" (that is, an Item set to the values for what I determine to be an "orange").  I'm adding the items to the Room.List.
 
I've realized what was going on (hinted at through my ramblings above...just typing it out helped me think through it).  Clone() is basically this:

public Item Clone()
{
Item c = new Item(console);
c.Name = this.Name;
c.Weight = this.Weight;
//etc.
c.ContentsList = new List<Item>(this.ContentsList);

return c;
}

...and then I can add them like so:

Room.List.Add(ObjectHandler.Create("orange"));

 
I thnk the part that confused you was that there's a list in the Item.  I'm not adding to that list here either...again, the list I'm adding to is Room's list.  The Item's list is so items can have there own inventory (bags can carry stuff, for example...or there might be a pearl in that orange blink.png).  And it's actually not just a List, but an "Inventory" object, which has the list.  Again, I've siplified some of the stuff to try to make my point easier to follow.
 
Here's the part I'm talking about: if I don't make c.ContentsList = NEW in that Clone(), and instead make it equal to this.ContentsList, that list will reflect all "clones" of the item.  This is because, I'm now assuming, that it's using the same reference and not actually making a "new" list.  The reason I though it would is because in an Item's constructor, the list = new List.  And since in Clone() it creates "Item c = new", I expected the list to be, naturally, a new list. 
 
Since changing Name or Weight in one copy doesn't affect the other copies, but changing the list does, I figure it's something to do with the way C# refers to things [pass-by-value vs pass-by-ref or something?]
 
Anyway, like I said, everything's good and working now.  I just wondered if that's how things work in the C# world.

Edited by Neith

Share this post


Link to post
Share on other sites

A few other notes on how to do some things in C#. First of all, you don't have to do the string.ToLower in the comparison yourself. You could create a Dictionary which uses a case insensitive string comparer. In the Add function, you don't have to check to see if a key exists before adding it. If your objective is to either add a new item, or replace an existing item, just assign the value via the index. You also don't have to search Dictionaries manually as you are in the create method. As ChaosEngine said, just use TryGetValue. This is what I would do. 

 

Here is an example showing the changes I would make.

 

public class ObjectHandler
{
  private Dictionary<string, Item> objects = new Dictionary<string, Item>(StringComparer.InvariantCultureIgnoreCase);


  public void Add(Item item)
  {
    objects[item.Name] = item;
  }


  public Item Create(string Name)
  {
    Item item;
    if(objects.TryGetValue(Name, out item))
    {
      return item.Clone();
    } 
    
    return null;
  }
}

I would also strongly consider changing the name from Create to CreateCopy. 

 

Edited by tstrimple

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!