Removing element from array

[power_computer]

Ok, so I am working on a function that remove a element from the array if the name entered by a user is = to a element in the array

It clears of the memory of said element but i need to re order the array

I searched on here but nothing seemed to cover it for an array of structs

for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {
                   songs[i].title.clear();
                   songs[i].artist.clear();
                   songs[i].genre.clear();
                   songs[i].time = 0;       
   
               songs[i-1].title = songs[i].title; 
               songs[i-1].artist = songs[i].artist;
               songs[i-1].genre = songs[i].genre;
               songs[i-1].time = songs[i].time;
         }

[VernonDozier]

Ok, so I am working on a function that remove a element from the array if the name entered by a user is = to a element in the array

It clears of the memory of said element but i need to re order the array

I searched on here but nothing seemed to cover it for an array of structs

for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {
                   songs[i].title.clear();
                   songs[i].artist.clear();
                   songs[i].genre.clear();
                   songs[i].time = 0;       
   
               songs[i-1].title = songs[i].title; 
               songs[i-1].artist = songs[i].artist;
               songs[i-1].genre = songs[i].genre;
               songs[i-1].time = songs[i].time;
         }

Close, but not quite. You just overwrote songs[i-1], which presumably may NOT be a duplicate. I assume you want to "delete" songs, by overwriting it with songs[i+1], like this:

for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {   
               songs[i].title = songs[i+1].title; 
               songs[i].artist = songs[i+1].artist;
               songs[i].genre = songs[i+1].genre;
               songs[i].time = songs[i+1].time;
         }
}

This isn't quite the solution. There are still problems with it and thus you need to do more, but you're no longer overwriting a song that wasn't a duplicate, so it's a start.

Note that the word "delete" is in quotes on purpose. Depending on how technical one wants to be, you're not really "deleting" or "removing" anything from the array. The array stays the same size. But you're changing the array in an organized way such that you have one fewer element that you care about (i.e. a value may still be there, but you couldn't care less what it is), so in effect you're deleting it.

Not sure if the above paragraph adds or detracts from the discussion. If it detracts, just ignore it for now. :)

[power_computer]

Well in this particular function, as seen in my previous post, you can recall the code

I call this function to ask the user to input a song title, then I sort through the array to see if there is a corresponding element in the array and remove that item from the array, then I "shorten" the array so It can print out the array properly when I call my print function, hence why I used

songs[i].title.clear();
songs[i].artist.clear();
songs[i].genre.clear();
songs[i].time = 0;

I thought it would clear out the memory, which I did, but I failed to shift the array.

However, you code make much more sense to simply shift the index of the array over 1 so that value gets overwrited if the user inputs a title that corresponds to an element in the array

Just as an FYI the user input is...asking the user to input the title of a song only.

As you said its it not the final solution because

(1,2,3,4) example of output

now after said "removal"

(1,2,4,4)

I just need to learn how to remove that duplicated element which seems to be the following element after the match
in titles

And I think I fixed it, I am bit abrupt on explaining my meaning but I did this to the "remove" function

int remove(music songs[10], int lCount)
{
    string name;
    int i =0;
    
    cout<<"Enter title: ";
    cin.ignore();
    getline(cin,name);
    
   for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {
                   
                   songs[i].title = songs[i+1].title;
                   songs[i].artist = songs[i+1].artist;
                   songs[i].genre = songs[i+1].genre;
                   songs[i].time = songs[i+1].time; 
         }
   }
   
return (lCount - 1);
}

and when I call this function I simple did

lCount = remove(songs, lCount);

So the iterator of "lCount"; which the current arrays size depending on the last file read gets reduced by 1

Not gonna lie it is a buggy in some instances, the remove and then print correct amount of array works some times the value does repeat

[kvprajapati]

struct can be easily copied using an assingment operator.

songs[0]=songs[1];

You can't remove an element from the an array. However you may assign an empty struct,

music empty={"","","",0};
songs[i]=empty;

[vmanes]

You're on the right track, but not really moving all the songs following the deleted item up. You only move the i+1 element to the i postion when the match was found. Everything past that will remain where it was.
That is, if the list is 1 2 3 4 5 6 and you want to remove 3, you'll get 1 2 4 4 5 6, but with the reduced count, item 6 won't be seen.

Better method is to iterate through the array till you find the item to remove, or run out of array (it's not found). When you find it, begin a loop that moves every remaining item up one position, starting from the location item was found to the end of array.

Oh - watch for reading past end of array. If your loop counter i points to the last element, what's at i+1?

Or, using the code you've written, use the comparison to set a flag if match is found, and use that flag to indicate if an item should be moved up.

bool move = false;
for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {
                move = true;  //never gets reset to false
                continue;  //we'll update on the next loop pass
           }

           if( move )
           {
                   
                   songs[i-1].title = songs[i].title;
                   songs[i-1].artist = songs[i].artist;
                   songs[i-1].genre = songs[i].genre;
                   songs[i-1].time = songs[i].time; 
         }
   }

[power_computer]

Any ideas to solve my last buggy issue where in some instances when I "remove" the first element in the array, the second element is copied into the cell block [0],[1]

[power_computer]

Your method works when i+1, i-1 doesnt seem work as well, however, as stated i+1 is still a bit buggy in some circumstances, it seems the boolean is checking well, because when enter anyname the next value in the array is removed and not displayed, hmmm

[vmanes]

Your method works when i+1, i-1 doesnt seem work as well, however, as stated i+1 is still a bit buggy in some circumstances

Look at it closely, be sure you have the continue statement as shown.

Also, your decrementing of the count that's returned should only occur when the flag "move" is true. Don't modify it if the target song was not found.

[power_computer]

It is still buggy, not removing properly when I imply your method

Here is my entire code

#include <iostream>
#include <fstream>
#include <string>

using namespace std; 

struct music
{
       string title;
       string artist;
       string genre;
       int time ;
};

int read(char* file, music songs[10] , int lCount = 0);
void print(music songs[10], int lCount);
int remove(music songs[10], int lCount);
int main()
{
    music songs[10];
    int j = 0, lCount = 0, choice = 0;
    char file[256];
    
    lCount = read("song.dat",songs);
    
   do
   {
    cout<<endl;
    cout<<"Enter choice: ";
    cin>>choice;
    
    switch(choice)
    {
                  case 1: 
                          cout<<"Enter new file: ";
                          cin>>file;
                          lCount = read(file, songs, lCount);
                          break;
                  case 2:
                         print(songs, lCount);
                         break;
                  case 3:
                         lCount = remove(songs, lCount);
                         break; 
                  default:
                          cout<<" ";
                          break;
    }
    }while(choice != 100);
   
   
    
    system("Pause");
    return 0;
}
    
int read(char *file, music songs[10], int lCount)
{
    
    
    ifstream fin;
    string string;
    int i = 0;
    
    for(int j=0; j < lCount; j++)
    {
            songs[i].title.clear();
            songs[i].artist.clear();
            songs[i].genre.clear();
            songs[i].time = 0;
    }
    
    
    fin.open(file);
    
    while(!fin.eof() && i < 10)
    {                
        getline(fin,songs[i].title);
        getline(fin,songs[i].artist);
        getline(fin,songs[i].genre);
        getline(fin,string);
        songs[i].time = atoi(string.c_str());
        i++;
    }


    fin.close();
    return(i);
}

void print(music songs[10], int lCount)
{
     
      for(int j = 0; j < lCount; j++)
    {
            cout<<songs[j].title
                <<songs[j].artist
                <<songs[j].genre
                <<songs[j].time<<endl;
                
    }
}
int remove(music songs[10], int lCount)
{
    string name;
    int i =0;
    bool move = false;
    
    cout<<"Enter title: ";
    cin.ignore();
    getline(cin,name);
    
   for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {       
                   move = true;
                   continue;
           }
           if(move)
           {
                   songs[i].title = songs[i-1].title;
                   songs[i].artist = songs[i-1].artist;
                   songs[i].genre = songs[i-1].genre;
                   songs[i].time = songs[i-1].time;
           }
   }
   
   
   
return (lCount);
}

The read and print functions work fine as far I tested them with 3 different files, and also the read function as to read from a song.dat file upon start up just wanted to start that

[vmanes]

You got the assignment backwards, should be

songs[i-1].title = songs[i].title; 
      //etc.

[power_computer]

Only works when I remove the last element in the array
I also added lCount--;
When the flag is trigged

argh

[VernonDozier]

The read and print functions work fine as far I tested them with 3 different files, and also the read function as to read from a song.dat file upon start up just wanted to start that

They may work, but if the files end with a line return after the last piece of data (which is quite common), they won't. Unless you have been told that there won't be a line return after the last piece of data, don't assume there won't be. It's more likely than not that there WILL be a line return. Do any of your three test files have one?

And as mentioned in your earlier thread, you should really have a better Print function. You need one anyway eventually, so why not write it now so it can help you debug?

Regarding your deletion code, you need to post the function each time (don't post the whole program each time). Otherwise, no one knows where you put everything. Words and code are better than just words.

[power_computer]

How can I improve my print function?

It seems when I subtract one when I call my function it works fine, so it seems to be a problem when i try decrement lCount in the function remove

case 3:
                         lCount = remove(songs, lCount) - 1;

However, when I do

return (lCount - 1);

Works fine so far, However this total bypasses the input check if its a valid element in the array, hmm

[VernonDozier]

How can I improve my print function?

Number Of Songs = 3

Song 1:
Title : Jump
Artist : Van Halen
Genre : Rock
Time : 300

Song 2 :
Title : Play
Artist : Jennifer Lopez
Genre : Dance/Pop
Time : 200

Song 3 :
Title : Beer For My Horses
Artist : Toby Keith/Willie Nelson
Genre : Country
Time : 250

Much cleaner. Problems stand out much more too. Line up the colons and it'll look even better and more presentable.

It seems when I subtract one when I call my function it works fine, so it seems to be a problem when i try decrement lCount in the function remove

case 3:
                         lCount = remove(songs, lCount) - 1;

However, when I do

return (lCount - 1);

Works fine so far, However this total bypasses the input check if its a valid element in the array, hmm

Both are incorrect. Your function needs to not subtract anything unless there is a match. So if you type in "Michael Jackson" and there are no Michael Jackson songs, nothing should be deleted. Hence lCount needs to stay the same in that case.

[power_computer]

I implemented it when there is a match along with my functions to move the array elements over by one, lCount doesnt seem to be keeping accurately

[power_computer]

Bump...Im thinking I just make the function a void, and do not return the lCount

[VernonDozier]

Regarding your deletion code, you need to post the function each time (don't post the whole program each time). Otherwise, no one knows where you put everything. Words and code are better than just words.

Bump...Im thinking I just make the function a void, and do not return the lCount

You're not getting responses because you're not posting the function in each post. We can guess what you're doing, but we don't know, so we can't help. If you don't return the count, how do you know how many elements are still in the array?

[power_computer]

I do return the count.

Here is the function code along with the function call

Function Definition

int remove(music songs[10], int lCount)
{
    string name;
    int i =0;
    bool move = false;
    
    cout<<"Enter title: ";
    cin.ignore();
    getline(cin,name);
    
   for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {       
                   move = true;
                   continue;
           }
           if(move)
           {
                    songs[i-1].title = songs[i].title;
                    songs[i-1].artist = songs[i].artist;
                    songs[i-1].genre = songs[i].genre ;
                    songs[i-1].time = songs[i].time;
                    lCount = lCount  - 1;
           }
           
   }
return(lCount);
}

Function Call

lCount = remove(songs, lCount);

[VernonDozier]

I do return the count.

Good. You should return the count. You were mentioning making it a void function so if that was the case, you wouldn't be returning anything, which would be bad. Have it return an int, as you do.

This is where you put debugging statements in, so you know what is happening. In particular you need to know when/how many times you go into each if block. How many times do you want lCount to decrement?

int remove(music songs[10], int lCount)
{
    string name;
    int i =0;
    bool move = false;
    
    cout<<"Enter title: ";
    cin.ignore();
    getline(cin,name);
    
   for(int i = 0; i < lCount; i++)
   {
           if(name == songs[i].title)
           {
                   cout << "i = " << i << " Title is the same\n";       
                   move = true;
                   continue;
           }
           if(move)
           {
                    songs[i-1].title = songs[i].title;
                    songs[i-1].artist = songs[i].artist;
                    songs[i-1].genre = songs[i].genre ;
                    songs[i-1].time = songs[i].time;
                   cout << "i = " << i << " Decrementing lCount\n"; 
                    lCount = lCount  - 1;
           }
           
   }
return(lCount);
}

It'll help you picture what's going on. If you aren't using a debugger, you should be adding debugging statements all over the place displaying values (remember to take them out later). It's one of the reasons I suggested putting in a better Display function. Add the red lines above. Run it a few times with different input and see if you see what's going on. Consider whether you want to take the decrementing out of the second if block and/or put a decrement line into the first if block.

[vmanes]

if(move)
           {
                    songs[i-1].title = songs[i].title;
                    songs[i-1].artist = songs[i].artist;
                    songs[i-1].genre = songs[i].genre ;
                    songs[i-1].time = songs[i].time;
                    lCount = lCount  - 1;
           }

ooooopppssss - lCount gets decremented way too many times! Only reduce it one time, if there was in fact a move. How about

if(move)
           {
                    songs[i-1].title = songs[i].title;
                    songs[i-1].artist = songs[i].artist;
                    songs[i-1].genre = songs[i].genre ;
                    songs[i-1].time = songs[i].time;
            }
     } //end of for loop

     if( move )
         lCount = lCount  - 1;

     return lCount;
}

[power_computer]

I tested it and I got

i = 1 Decrementing lCount
i = 2 " "

Then went to enter some more input the program began a infinite loop when I removed the first element in the array, same thing If I move lCount - 1 to the first if block

Vmanes idea works, THANKS!!! I would have been stuck there forever, instead ok taking a solution and running I want to know why it got repeated more than once inside the for loop

[Lerner]

Go back to post #19. Add the lines in red to your original code and add another debugging line after this one:

lCount = lCount - 1;

that looks like this;

cout << "lCount = " << lCount << endl;

Recompile and run. Observe the power of debugging statements (if you feel comfortable use a debugger you can almost always do something very similar with that type of software as well.)

My suspicion is that the continue statement didn't break you out of the for loop. Therefore you end up decreasing lCount every time you go through the loop after you find the value you want to delete. For that reason I don't like to use continue statements or break statements in complicated loops. Instead I set the dependent variable to the value that will stop the loop. In this case I would replace the continue statement with the line:

i = lCount;

I also think it's best to have loops do one thing only, whenever possible. In this case I would have written the code to:

1) loop through array looking for desired value, stopping when/if found.
2) if desired value is found decrease lCount outside of loop
3) use a second loop to shift elements with index greater than the index of desired element to delete to left by one.

But to each their own.

Learning how to use a debugger or effectively use debugging statements is a very useful skill to have.

[vmanes]

<snip>
I also think it's best to have loops do one thing only, whenever possible. In this case I would have written the code to:

1) loop through array looking for desired value, stopping when/if found.
2) if desired value is found decrease lCount outside of loop
3) use a second loop to shift elements with index greater than the index of desired element to delete to left by one.

But to each their own.

Learning how to use a debugger or effectively use debugging statements is a very useful skill to have.

I made essentially that same suggestion in my first reply, and if I was starting from scratch that's how I'd approach it. But then, since the OP already had code that nearly solved the problem, I showed how to correct that code.

There's almost always more than one way to correctly solve a coding problem. That's what makes it fun and interesting.

[emirpp]

why don't you use linked-list?
its easy to remove element in the middle,,
you just need to shift the pointer from the previous item to the next item from the element you deleted..
[3]-> [4]-> [5] become [3]->[5]