FEATURE: Add pop(key) method

[igorissen]

🦄 Context

When we first used this plugin, we wondered if there was a way to retrieve and delete a value. We found in the documentation that the get method accepts an additional parameter, which is the delete option after retrieval.

For new developers on the project or people who will be reviewing the code, a recurring question we've noticed is what the second parameter of the get method does?

It could be great to have a method with more clarity like Array.prototype.pop() to retrieve and delete a value.

🤖 Proposal

Add a new method with the name pop which retrieves and removes the value associated with the specified key from the request yar store.

🌈 Notes

N/A

💯 Tests

1. Run the commands npm test and/or npm run test-cov-html
2. Check tests are all ✅

[kanongil]

Thanks for the PR.

While I agree that the clear option for get() has understanding issues when reading the code, your fix is not the correct solution (supposing it is a big enough issue to warrant a fix).

pop() is normally used on arrays (with no argument), and using it for a key-value store adds confusion. Additionally, you would expect to have the counterpart push() available.

If we want to fix this, the simple solution is probably to change it to an option argument with a clear parameter. Used like:

request.yar.get('key', { clear: true });

The even simpler solution, is to remove the option all together, requiring users to explicitly call clear(). The option is a pure developer convenience:

request.yar.get('key');
request.yar.clear('key');

I'm probably in favour of this approach.

While we are at it, we might want to rename clear (both as an option, and the method) to delete, to better match the Map semantics, where clear() removes all elements.

[igorissen]

The even simpler solution, is to remove the option together, requiring users to call clear explicitly ().

While we are at it, we might want to rename clear (both as an option and the method) to delete, to better match the Map semantics, where clear() removes all elements.

I'm also in favour of the removal of the options parameter. To have a simple function with one goal will be simple to understand and even simpler to test. The developers could create themselves a function which calls the get and delete.

@kanongil do I need to create a new feature request issue or update my current feature request issue? or should I do the modification and create a PR?