48 allow update before remove

[luoto]

Here is how I thought to solve this problem:

• If a change object or function was passed into remove, then remove will either extend it with a the property _deleted: true, or put a thin wrapper around it, which appends the _deleted: true property to the object before it gets passed into the actual change function.
• If no change arguments were passed, then _deleted: true will be appended to each of the objectsOrIds before they are sent off to the update functions without the change parameters, allowing them to update and 'delete' properly.

I also updated the function signature to accept an optional change argument and made changes to the documentation to reflect that.

closes #48

[gr2m]

@luoto that is great work 👍 And I love that you also updated the comments for the docs, I tend to forget that myself all the time :)

I think we can simplify the code a little. For example, you call updateOne, updateMany and Array.isArray twice in the code now. Do you think we could reduce that?

Maybe you can also reuse the utils/change-object.js method to apply the optional change property, if passed.

For example, we could leave the remove function as is, but remove the {_deleted: true} arguments, and instead wrap objectsOrIds in a markAsDeleted method, for example like this:

function remove (objectsOrIds, change) {
  return Array.isArray(objectsOrIds) ?
    updateMany.call(this, objectsOrIds.map(markAsDeleted.bind(null, change))) :
    updateOne.call(this, markAsDeleted(change, objectsOrIds))
}

That way we have a single method that gets called separately on every object / id, no matter if we got an array or not.

Then in the markAsDeleted, we could normalise the objectOrId so we always get an object that we can extend with {_deleted: true}. And then, if change is passed, apply it using the existing changeObject method.

I've made a gist of the markAsDeleted method, but maybe you want to give it a try first? Your choice :) Here is the gist

I really hope that the my guidance is helpful, please let me know if there is any way I could do better, it's a first-timer for me as well ;)

[luoto]

@gr2m Here is what I came up with your guidance. It is very similar to what you had. Which is so much more elegant than what I had originally.. haha.

Thank you so much for your time. I recognize that this could be taking up a lot of it, and I will use it as wisely as I can :). Trust me, I have already learned so much in this process so far. For this refactored version, I kept thinking to myself, 'wow this is so slick!'. I will have to try use to these ternary operators and functional style of programming more often, and of course code reuse ;)

I also noticed from the slack chats that you do some pair programming with some of the other contributors. Maybe I could sit in some time just to observe and learn more about the project?

In the end, lets go with what you had in your gist because to me, it looks more consistent with the project.

I'm going to make the changes and update the PR for your review. Thanks.

[varjmes]

@luoto just to butt in: never worry about 'taking up time', Hoodie is for everyone and if we didn't talk to/help people we would not be Hoodie :D

[gr2m]

@luoto wow you really came very close to my solution, that is a very good sign, must be good then :)
I've added a comment to your gist here, on why I use extend instead of setting object._deleted property direcly: https://gist.github.com/luoto/46570c0a230d4940e402 let me know if that makes sense to you.

Can you update your PR then?

[luoto]

@gr2m: I see the intentions now :) In case you want to see my comments.

I'll try to find you on slack. I thought I had already pushed the changes.

@Charlotteis: thanks for the advice :)

[kentcdodds]

Welcome to the open source community @luoto :-D

[luoto]

Thanks @kentcdodds :). It feels great to be a part of something greater. Time to hunt down and work on another issue, huzzah! ;)

[kentcdodds]

👍

[NickColley]

🎉 nice one @luoto 💃