Nahuel Leiva
Nahuel Leiva

Reputation: 117

Modifying a copy of an array is NOT causing the original object to change

I have a class where I manage some arrays, they get filled with data that I get from an API I consume. These lists represent clients by state, depending on their status I store these clients in their corresponding list. For privacy reasons I cannot share the entire code with you, but I can extract some things to help you understand my problem.

Let's assume the following where I initialize said arrays like this:

   this.waitingClientsList = [];
   this.usersAttendedList = [];
   this.absentClientsList = [];
   this.suspendedClientsList = [];
   this.announcedClientsList = [];
   this.clientsBatch = [];

Each list will start empty.

To avoid repeating too much code, I manage them with a dictionary like this:

   this.listsByState = {
      "waiting": this.waitingClientsList,
      "suspended": this.suspendedClientsList,
      "absent": this.absentClientsList,
      "inattention": this.usersAttendedList,
      "invideocall": this.usersAttendedList,
      "announced": this.announcedClientsList,
      "vdconnectingerror": this.usersAttendedList,
      "videocallended": this.usersAttendedList,
      "vdmissedcall": this.usersAttendedList
    }

To update their content I have a function that checks an API method that works with pagination, retrieves the data and stores it in their respective list like this (this is an example):

private GetWaitingClients(state, nextPage, id, pageSize?, pageNumber?) {
    this._requestService.getWaitingClients(state, id, pageSize, pageNumber).then(async (response: any[]) => {
      // Getting the list by state
      let clientList = this.listsByState[state];
      // if we are changing the page, we should empty the respective list
      if (nextPage) {
          clientList = [];
      }

      response.map(item => {
        // If the client doesn't exist, it pushes it to the list
        // otherwise I manipulate the client
        if (clientList.some(obj => obj.virtualSpaceId == item.virtualSpaceId)) {
          // I do something here if the client exists
        } else {
          // If not, I push it to the list
          clientList.push(item);
        }
      });

      [...]
      // More stuff that I do
    }).catch(error => {
      // Here I print the error in case there was a problem with the API request
    });
  }

I retrieve each list with this line of code let clientList = this.listsByState[state];, this is the way I found to avoid doing a switch/case statement or multiple if/else statements. So far, this works fine. If there's a new entry in the database, both the local variable clientList and their referenced array update correspondingly, no problems there.

I have the method in the API built in a way that the first time the page loads it will return the first page with 5 results. If I change the page on the front-end, the function kicks in and should retrieve the next 5 results, so on so forth.

The problem is that when I change the page the variable clientList does get updated if I change its content but the original referenced array does not, which is bugging me. As far as I understand, when doing this assignment let clientList = this.listsByState[state]; I assume that clientList is pointing to the same object that its corresponding list in memory but that's not happening here so I don't know where is my problem. I also tried emptying the clientList variable before doing the map operation but it does nothing, the original array remains unchanged.

The only way I can manage to change the referenced array content is by doing a direct assignment like this, for example: this.waitingClientsList = clientList, but this invalidates my previous code and that lefts me with no choice but to do a big and ugly switch/case statement or multiple if/else statements which I don't want to do. I'm trying to keep my code as clean as possible.

Any suggestions? Thanks in advance!

Upvotes: 2

Views: 1205

Answers (3)

Nahuel Leiva
Nahuel Leiva

Reputation: 117

First thanks both @ghybs and @Mathew Breg for the suggestions.

First, I removed the clientList = [] assignment and I used clientList.length = 0 instead which works perfectly as both of you pointed out. I've found another SO question where it states that clientList = [] is, actually, the fastest way to empty an array if you don't have references to the original one, otherwise the original will remain unchanged. That was my first mistake.

The second mistake is (and it's also something I didn't mention before and it needs a rework) that I currently have a subscription to a service that controls the page visibility that I was not handling correctly. Every time my page gained focus the original array will be populated entirely, and it would be shown as unchanged. Once I commented that piece of code and in conjunction with clientList.length = 0 and some corrections here and there, everything started to work flawlessly.

Another thing I've added to the code that helped me to refresh the table view (a thing to mention I forgot => to display the results, I'm currently using MatTable and MatPaginator for the data pagination from Angular Material) is the renderRows() function from the MatTable I use to display the data on the page after I change the page index.

I would love to mark both comments as the answers to my question because despite this simple line of code clientList.length = 0 was the correct way to manipulate the original array reference, the problem was somewhere else in my code after doing some more tests and debugs. For now, I'll mark the answer from @Mathew Breg but I want you to know that both of you were correct.

Upvotes: 3

ghybs
ghybs

Reputation: 53320

That is a classic JS mistake:

  • assign the reference to an object into a new variable (typically to shorten the code) var short = myObject.some.long.path.or[dynamicKey]
  • make a deep change => the original object is correctly modified short.subProperty = 2; myObject.some.long.path.or[dynamicKey].subProperty === 2;
  • re-assign the variable and expect the original object to be modified accordingly => disappointment :-( short = null; myObject.some.long.path.or[dynamicKey].subProperty === 2;

(In the example I used an object, but it is the same for an array)

In your case, since your test dynamic key is always in scope, you can simply keep on using it whenever you want to access the corresponding list: this.listsByState[state] = [];

Yes it may look redundant / less readable, but you now have a working code at least.

Now to correctly use your new clientList variable and keep on modifying the original list, as shown above and implied by MathewBerg's answer, simply make sure to always use a deep property or function that mutates the object in place:

clientList.length = 0; // works
clientList.splice(0, clientList.length); // to empty

clientList.push("itemAtTheEnd"); // works
clientList.unshift("itemAtTheStart"); // works
clientList.splice(2, 0, "insertedItem"); // to insert after the 2nd item

clientList.pop();
clientList.shift();
clientList.splice(3, 1); // to remove the 3rd item

You can easily enforce this rule in your code by preventing re-assignment:

// Initialize a const variable to forbid re-assignment
const clientList = this.listsByState[state];

Upvotes: 1

Mathew Berg
Mathew Berg

Reputation: 28750

  // Getting the list by state
  let clientList = this.listsByState[state];
  // if we are changing the page, we should empty the respective list
  if (nextPage) {
      clientList = [];
  }

Are you expecting this line of code to also reset the array for this.listsByStatep[state]? It won't do that as you've assigned a new array to clientList and so the reference isn't the same (you can check this by doing console.log(clientList === this.listsByStatep[state]) before and after you assign the []). To do what you're looking for change it to this:

  // Getting the list by state
  let clientList = this.listsByState[state];
  // if we are changing the page, we should empty the respective list
  if (nextPage) {
      clientList.length = 0;
  }

This doesn't destroy the reference and empties the array.

Upvotes: 1

Related Questions