tags:

views:

114

answers:

6

Hi, I'm fairly new to C# so this might be difficult for me to put into words.

I am creating a media application and I have a class (called MediaFile) that contains information about media files (name, size, play count and tags). These are of string, double, int and List<string> types, respectively.

I have a list of these objects, so for example to call MediaFile[2].Tags will refer to a list of "tag" strings (related keywords).

The problem I have is that when I ask the user to enter tags for the selected MediaFile, whenever I try to save those tags to that specific object, the tags get saved to every single object. The code for the assignment currently looks something like this:

MediaFile[lstLibrary.SelectedIndices[0]].Tags.Add(tempTag);

'tempTag' is a string that I'm trying to add into the list of strings, but like I said - even though only one file is selected, the 'tempTag' string gets added into the list of strings of each MediaFile.

Can anyone shed some light on what I'm doing wrong?

Many thanks.

EDIT: Thanks for all of your responses. Whenever I create a MediaFile instance I pass new List<string> to the constructor. Then later on when I go to change this list of strings I discover all of the MediaFiles seem to have the same string reference. Here is the MediaFile class:

public static List<MediaType> MediaFile = new List<MediaType>();
public class MediaType
    {
        public string Name;
        public string Path;
        public double Size;
        public int Plays;
        public List<string> Tags;

        // Constructor for adding new files
        public MediaType(string path, List<string> tags)
        {
            Path = path;
            Tags = tags;
        }

And after I ask the user to select a file to add to the media library:

MediaFile.Add(new MediaType(Path.GetFullPath(file), new List<string>()));

So there are no 'tags' at first, but then later on (and herein lies my problem):

if (tempTag != "")
    MediaFile[lstLibrary.SelectedIndices[0]].Tags.Add(tempTag);
}

Any idea? Apologies this post is so long!

A: 

Where do you put stuff in MediaFile? How do you do it?

My guess is that you're inserting the same MediaFile object several time in the list. So, when you modify one, you end up modifying all of them because, after all, they are the same object.

Etienne de Martel
+1  A: 

That suggests you've added the same string reference to all the MediaFiles. In other words, you might have done:

List<string> tags = new List<string>();

for (int i = 0; i < 100; i++)
{
    MediaFile file = new MediaFile(tags);
    MediaFiles.Add(file);
}

This is very easy to test - just check whether MediaFile[0].Tags == MediaFile[1].Tags - I suspect you'll find that evaluates to True, meaning you're using the same list for both files.

Instead, you should create a new List<string> for each MediaFile instance.

If you could post some code, that would help us to pin down the problem more precisely, of course...

Jon Skeet
Thanks for your response Jon, I've edited my original post to provide some extra information.
Zach Whitfield
@Zach: Okay, well to start with you need to stop using public fields :) But it doesn't explain the behaviour you're seeing. If you could prune the code down to a short but complete example, that would really help. See http://tinyurl.com/so-hints
Jon Skeet
+1  A: 

It looks like you have set all of the MediaFiles[i].Tags to be references to the same instance of a list.

Kragen
A: 

It sounds like you've set all the objects in your list to use the same instance of the Tags list

w69rdy
A: 

I suspect that you're initialising your objects incorrectly. You're probably (effectively) creating them like this:

List<string> tags = new List<string>();

MediaFile m1 = new MediaFile();
m1.Tags = tags;

MediaFile m2 = new MediaFile();
m2.Tags = tags;

That is, the Tags property of each MediaFile refers to the same actual List<string> object. Thus, when you add a string to m1.Tags, you're also adding it to m2.Tags.

What you want to do instead is:

MediaFile m1 = new MediaFile();
m1.Tags = new List<string>();

MediaFile m2 = new MediaFile();
m2.Tags = new List<string>();
teedyay
A: 

Does your Tags property have a set accessor?

Or do you have code that looks (something) like this?

var tags = new List<string>();
for (int i = 0; i < MediaFile.Count; ++i)
{
    MediaFile[0] = new MediaFile(tags);
}

If so then each of your MediaFile instances is sharing the same internal List<string>.

You should not have a set accessor on this property at all, actually, nor should the MediaFile constructor be willing to take some external List<string> (unless it's going to copy it). Instead assign it only in the constructor for MediaFile to a new List<string>(); this way you will ensure that each instance gets its own list:

public MediaFile()
{
    this.Tags = new List<string>();
}

Or:

public MediaFile(IEnumerable<string> tags)
{
    this.Tags = new List<string>(tags);
}
Dan Tao