2

I am trying to build a 3-tiered function: First, an array lists available workshops (array is called 'workshops'). Second, another array lists workshops that a user has selected (this array is called 'selectedWorkshops'). Third, I have a final array called 'registeredWorkshops'.

When my function is run, I want objects within 'selectedWorkshops' to be added to 'registeredWorkshops', then I want to delete any objects within 'selectedWorkshops' from both 'selectedWorkshops' and any matching elements from 'workshops'. So, where those objects used to exist in both 'selectedworkshops' and 'workshops', now they only exist in 'registeredWorkshops'.

Here's what I've got so far:

addRemoveWorkshops = function(){
    var numberOfWorkshops = selectedWorkshops.length;
    for(var i = 0; i < numberOfWorkshops; i++ ){
        registeredWorkshops.push(selectedWorkshops[i]);
        for(var j = 0, arrayLength = workshops.length; j < arrayLength; j++) {
            var searchTerm = selectedWorkshops[i].WorkshopId;
            if (workshops[j].WorkshopId === searchTerm) {
                workshops = workshops.slice(j);
            }
        }
        selectedWorkshops = selectedWorkshops.slice(i);
    }
};
addRemoveWorkshops();

However, the function doesn't appear to work properly. It doesn't seem to be deleting the correct workshops, and it only seems to add one of the selectedWorkshops to registeredWorkshops. What am I doing wrong?

Here's a codepen demonstration: http://codepen.io/trueScript/pen/GgVWMx

Improve this question
3
  • I think you cannot invoke array.slice(i) or array.slice(j), when i or j value is zero Commented Apr 14, 2015 at 15:46
  • 1
    @TheGuest You can, but it just returns an empty array. Commented Apr 14, 2015 at 16:15
  • @DanPrince I mean in this case Commented Apr 14, 2015 at 16:16

4 Answers 4

Reset to default
2

If it's not possible to add other properties to the objects (as per my other answer) then I'd tackle it like this:

function registration(workshops, selected, registered) {

  // add the selected workshops to registered
  selected.forEach(function(workshop) {
    registered.push(workshop);
  });

  // remove them from the other lists
  registered.forEach(function(workshop) {
    removeWorkshop(selected, workshop);
    removeWorkshop(workshops, workshop);
  });

}

function removeWorkshop(list, workshop) {
  var index = list.indexOf(workshop);

  if(index >= 0) {
    list.splice(index, 1);
  }
}

The function expects each of the arrays to be passed in as arguments and it will modify them in place. Things always become clearer and easier to test if you move your loops out into functions before nesting them.

There should be no reason not to use the indexOf method here, as it saves you having to write an extra loop. However, if for some reason you needed to use the WorkshopId property to locate the item within the list, you could create another helper method to do this for you.

function findWorkshop(list, workshop) {
  for(var i = 0; i < list.length; i++) {
    if(list[i].WorkshopId === workshop.WorkshopID) {
      return i;
    }
  }

  return -1;
}

Then you just amend the removeWorkshop function to reflect that.

function removeWorkshop(list, workshop) {
  var index = findWorkshop(list, workshop);
  list.splice(index, 1);
}
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

This is exactly what I'm looking for. My only problem is that, when added, it seems to skip over deleting the 'oranges' object in the 'workshops' array, and deletes 'pineapples' instead. I'm trying to figure out why. Any insight? Codepen: codepen.io/trueScript/pen/GgVWMx
Ahh. My bad. Turns out that .splice considers negative indexes to be relative to the end. I'll update the answer.
The other thing that probably needs changing is that if you are defining the workshop objects in the file like that, then you will need the findWorkshop function instead of using indexOf.
@Dan Prince Yep, that's why I suggested to store references in selectedWorkshops to workshops item rather than cloning them, in order to use indexOf.
Ultimately, I suspect that will be fine, when data is actually flowing in the application, but all the while the lists are statically defined at the beginning, (unfortunately) indexOf won't work.
2

I think it would be easier to slightly rethink your data structure. If you go for the imperative solution above, you run this risk of ending up with duplicate values in more than one list.

Would it not be easier to add registered and selected properties to your workshop objects?

var workshops = [
  {
    name: 'apples',
    WorkshopId: '19',
    registered: true,
    selected: false
  },
  {
    name: 'oranges',
    WorkshopId: '3b',
    selected: true,
    registered: false
  },
  // ...
];

Then if you need to be able to get a list of all the registered workshops, you can create it using a filter.

// helper function for filtering based
// on a given property
function property(name) {
  return function(object) {
    return object[name];
  }
}

var registered = workshops.filter(property('registered'));
var selected = workshops.filter(property('selected'));

To select a workshop, all you need to do is change the select property to true:

workshops[3].selected = true;

You could then write the original function to register all workshops that were selected like this:

function registration(workshops) {
  workshops.forEach(function(workshop) {
    if(workshop.selected) {
      workshop.registered = true;
      workshop.selected = false;
    }
  });
}
Improve this answer

1 Comment

Unfortunately, I can't do that in this situation (though it would definitely be easier). Thanks, though. Good answer.
1

A while loop + a for one :

var workshops = [{
    name: 'apples',
    WorkshopId: '19'
}, {
    name: 'oranges',
    WorkshopId: '3b'
}, {
    name: 'pears',
    WorkshopId: 'x6'
}, {
    name: 'pineapples',
    WorkshopId: '55'
}, {
    name: 'watermelons',
    WorkshopId: '8v'
}];

var selectedWorkshops = [{
    name: 'oranges',
    WorkshopId: '3b'
}, {
    name: 'watermelons',
    WorkshopId: '8v'
}, {
    name: 'pears',
    WorkshopId: 'x6'
}];

var registeredWorkshops = [];
var numberOfWorkshops;

addRemoveWorkshops = function () {
    numberOfWorkshops = selectedWorkshops.length;
    // A single while statment is enough and lighter
    while (selectedWorkshops.length) {
        var removedWorkshop;
        numberOfWorkshops = registeredWorkshops.push(selectedWorkshops[0]);
        for (var i = 0; i < workshops.length; i++)
        if (workshops[i].WorkshopId == selectedWorkshops[0].WorkshopId) {
            workshops.splice(i, 1);
            break;
        }
        selectedWorkshops.splice(0, 1);
    }
};
addRemoveWorkshops();

// Better for viewing the content (in firefox I have just "Object") : 
console.log("workshops : ");
for (var i = 0; i < workshops.length; i++)
console.log('- ' + workshops[i].name);

console.log("selectedWorkshops : ");
for (var i = 0; i < selectedWorkshops.length; i++)
console.log('- ' + selectedWorkshops[i].name);

console.log("registeredWorkshops : ");
for (var i = 0; i < registeredWorkshops.length; i++)
console.log('- ' + registeredWorkshops[i].name);
Improve this answer

4 Comments

When I run this, it successfully removed objects in the 'selectedWorkshops' array. However, it didn't successfully remove the correct items from the 'workshops array'. Instead, the remaining array items are 'apples' and 'oranges', despite oranges also being found in the registered workshops array. Please edit your question to fix this, if you can. Except for it not working, it's the closest thing to a direct answer to my question.
My bad, got it... indexOf is working with a strict equality. oranges from workshops and selectedWorkshops are not the same object, but identicals. Sorry for this mistake. I'm writing a quick fix.
@saestris : Updated. Anyway as @Dan Prince said, it's better to refine your data structure : if you can't follow his advice, I suggest this : instead of storing clones of selected objects in selectedWorkshops , you should put references. For example : selectedWorkshops.push(workshops[2]). or write a function : selectedWorkshops.push(getWorkshopById('8v'));
@saesris (Note : my advice was to allow the use of indexOf, wich works with strict equality). Dan Prince's answer sounds better though.
0 
addRemoveWorkshops = function(){
var numberOfWorkshops = selectedWorkshops.length;
for(var i = 0; i < numberOfWorkshops; i++ ){
    registeredWorkshops.push(selectedWorkshops[i]);
    for(var j = 0, arrayLength = workshops.length; j < arrayLength; j++) {
        var searchTerm = selectedWorkshops[i].WorkshopId;
        if (workshops[j].WorkshopId === searchTerm) {
            workshops = workshops.splice(j,1);
        }
    }
    selectedWorkshops = selectedWorkshops.splice(i,1);
}

};

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.