-1

I have to make a playlist where I can add a song title, the artist and the duration. I may not add the same song twice ( title, artist, and duration the same) and when i try to add a song with the same title and artist, but with a different duration, I have to keep the one with the longest duration.

This is my code I have now: 

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }

    private void btnAdd_Click(object sender, EventArgs e)
    {
        if (String.IsNullOrEmpty(txtTitle.Text) || String.IsNullOrEmpty(txtArtist.Text) || String.IsNullOrEmpty(txtDuration.Text) || System.Text.RegularExpressions.Regex.IsMatch(txtDurationr.Text, "[^0-9]"))
        {
            MessageBox.Show("No valid entry");
        }

        else
        {
            String title = Convert.ToString(txtTitle.Text);
            String artist = Convert.ToString(txtArtist.Text);
            double duration = Convert.ToDouble(txtDuration.Text);

            if (listBox.Items.Contains(txtTitle.Text) && listBox.Items.Contains(txtArtist.Text) && listBox.Items.Contains(txtDuration.Text))
            {
                MessageBox.Show("This already exists");
            }
            else
            {
                listBox.Items.Add("Title: " + title + ", Artist: " + artist + ", Duration: " + duration);
            }

        }
    }

    private void btnReset_Click(object sender, EventArgs e)
    {
        listBox.Items.Clear();
    }
}

The thing i did to check for duplication's doesn't work, and I have no idea how I could do the thing when i have the same title and artist but different duration.

But the check for the duplication's is the most important

Thanks in advance

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
Sam
  • 1
  • 2

4 Answers4

0

You're currently checking for the presence of any of those values (title, artist, duration), anywhere in your collection of items. Moreover, there'd have to be an exact match (an item that exactly matches the title, and an item that exactly matches the artist, etc).

You'll never find such a match, and your "duplicate detection" will never get a hit.

if (listBox.Items.Contains(txtTitle.Text)
    && listBox.Items.Contains(txtArtist.Text)
    && listBox.Items.Contains(txtDuration.Text))

Instead, check for any single item which contains all the values you're testing for:

if (listBox.Items.Cast<string>()
     .Any(x => x.Contains(title) && x.Contains(artist) && x.Contains(txtDuration.Text))

The cast to a string is necessary, since Items can be a collection of any object, but you're currently using strings.

It'd be cleaner if you created a class to define a "song" and used that to populate your ListBox instead. (There are properties on the ListBox to define which property is displayed to the user.)

public class Song
{
    public string Title { get; set; }
    public string Artist { get; set; }
    public double Duration { get; set; }
}

Then you could perform a similar check to the above, but cleaner:

if (listBox.Items.Cast<Song>()
     .Any(x => x.Title == title && x.Artist == artist && Duration == duration)
Grant Winney
  • 65,241
  • 13
  • 115
  • 165
0

Question: Sam, why don't you remove the duration check? Is this really necessary if you already check the title and artist? Maybe you change the if conditions?

Simple solution:

if ((listBox.Items.Contains(txtTitle.Text) && listBox.Items.Contains(txtArtist.Text) || 
((listBox.Items.Contains(txtTitle.Text) && listBox.Items.Contains(txtDuration.Text)))
{
        MessageBox.Show("This already exists");
}
else
{
      listBox.Items.Add("Title: " + title + ", Artist: " + artist + ", Duration: " + duration);
}

Another minor improvement of your code: Use String.ToUpper or String.ToLower, so you can avoid the checks such as Title1 and TiTlE1.

Odrai
  • 2,163
  • 2
  • 31
  • 62
  • That second condition does nothing. If the second condition is true then the first condition is true. – paparazzo Mar 05 '15 at 19:20
  • True, maybe Sam can remove the duration check? @Sam? – Odrai Mar 05 '15 at 19:22
  • That does not deal with max duration – paparazzo Mar 05 '15 at 19:26
  • Still does not deal with when i try to add a song with the same title and artist, but with a different duration, I have to keep the one with the longest duration – paparazzo Mar 05 '15 at 19:29
  • That is indeed a choice that Sam has to make. He can overwrite or maintain the old one. – Odrai Mar 05 '15 at 19:32
  • Thx guys :) Well i'm not that good at programming, as you could see .. But I managed to get it working, thx again :) Well the thing with the longest song, is not the most important, it was like a bonus, I'm happy that the duplicate thing is working fine now :) – Sam Mar 05 '15 at 19:32
  • Well it was with the code some guy wrote here, but I see he allready deleted it .. I just changed my If statement with this one: if (listBox.Items.Cast() .Any(x => x.Contains(title) && x.Contains(artist) && x.Contains(txtDuration.Text))) This did the job for me .. :) – Sam Mar 05 '15 at 19:35
  • @Sam Why do you give the check to an answer that does not work? That is not going to help other users. – paparazzo Mar 05 '15 at 19:42
  • @Sam maybe you can put the solution(s) in a new post :) – Odrai Mar 05 '15 at 19:44
  • @Blam because it was also helpful in understanding my fault .. Well I'm new here so sorry for creating indistinctness.. :/ – Sam Mar 05 '15 at 21:01
0

The first check is to see if the title and artist already exist. Use a for loop to loop through the items if needed. If yes, then store that list box item's text to a string and extract the portion which has a duration to another string variable. Convert the variable to a double using convert.Todouble. Then perform the next check, if this double value was smaller than the double variable 'duration'. If yes, then add to the listbox.

Asha
  • 25
  • 1
  • 7
0

By changing my if statement with this:

if (listBox.Items.Cast() .Any(x => x.Contains(titel) && x.Contains(artiest) && x.Contains(txtDuur.Text)))

My problem was solved

Sam
  • 1
  • 2