Add the ability to specify a comparator for array.unique() for objects only#860
Add the ability to specify a comparator for array.unique() for objects only#860DavidTPate wants to merge 1 commit intohapijs:masterfrom DavidTPate:issue-859
array.unique() for objects only#860Conversation
…paring objects only. This doesn't include comparing other types with a comparator becuase it would result in a performance decrease based upon how the comparison currently occurs.
|
I don't see why it would only compare objects. Also a simple function should be enough. |
|
@DavidTPate thoughts about my last comment ? |
|
@Marsup Sorry, been out-of-pocket for a bit. The reason why I only did it for objects was because I would need to change how the compares work for non-objects pretty considerably and also reduce the current performance quite a bit (at least that's what I thought at the time). Looking at this again, I think I was just being a bit of an idiot. I'll go through and fix some of those references as it should be that this only compares Arrays which is done in this block while non-Arrays are checked in this block. The second block I would need to reduce the performance of in order to support this functionality since right now it uses an Object Map to keep track of uniqueness based on type. I didn't do the less performant part because I expected it would be rejected, but I'll put that together so we can see what the whole change set would be and determine the action from there. |
|
Borrowed some of your code in 038c8ff, thanks ! |
|
Sounds good, thanks @Marsup It's been crazy for me here. |
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
I was looking at #859 and thought that it seemed like something that belonged in the core of Joi, not as a plugin/extension and had a pretty good idea of how I would implement it. This PR contains that initial implementation.
What It Does
It allows a user to specify a comparator function which is used only for objects.
In the event that a comparator isn't provided it just defaults to the standard behavior using
Hoek.deepEqual()so the change has a default and also is backwards compatible.What It Doesn't Do
It doesn't allow a comparator to be provided for types other than
object. Originally I wanted to make the parameter justcomparatorand use it for everything, but once I started implementing this I realized I couldn't do that without a noticeable decrease in performance because of how it works right now using an object map.The only cases I can think of where a comparator function would be useful for things other than objects is when the data needs to be modified, which that could just be done prior to validation. For example, if an array of numbers needs to have
Math.floor()run on each value and in that case I imagine you would want the value to come out of validation already run throughMath.floor()anyways. Others would be string manipulation and such like lowercase comparisons, etc.