2
\$\begingroup\$

So I want ask if I'm doing it right way.

Im still learning..

I want check textarea duplicates and alert if duplicate and remove it.

$(document).ready(function(){
 $('#lol').keypress(function(){
 var data = $('#lol').val().split('\n');
 var result = data.filter((word, i, arr) => arr.indexOf(word) === i);
 $('#lol').val(result.join('\n'));
 data.unique();
 });
});
(function() {
 "use strict";
 Array.prototype.unique = function unique() {
 var self = this;
 return self.filter(function(a) {
 var that = this;
 // console.log(that);
 if(!that[a]) {
 return that[a] = true;
 } else {
 alert("duplicate");
 return that[a] = false;
 }
 }, {});
 }
})();
asked May 19, 2019 at 1:05
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Take care extending built in objects

  • Extending the prototype of builtin objects should be done with care. The best way is via Object.defineProperty and first check if something has not already done so. (See 2nd rewrite)

  • Why create two copies of this.. self and that. Why not just use this and create a map object with a meaningful name. (see 2nd rewrite)

  • Don't add functionality outside the role of the function. Array.unique Defiantly should not be calling an alert (in fact you can not trust alerts (clients can turn them off) so you should never use them)

  • You have actually repeated the process of removing duplicates in the keypress handler. You could just have checked if the length of data and result are different and alerted the client from that. (see 1st rewrite)

Rewrites

First quick rewrite showing you don't need Array.unique

$(document).ready(function(){
 const lol = $('#lol');
 lol.keypress(function(){
 const data = lol.val().split('\n');
 const result = data.filter((word, i, arr) => arr.indexOf(word) === i);
 lol.val(result.join('\n'));
 result.length !== data.length && (alert("duplicate"));
 });
});

The second rewrite showing a safe way to add to inbuilt prototypes.

Note that the options configurable, enumerable, writable are set to false. Particularly you MUST set enumerable to be false or you could end up with all sorts of bugs appearing in existing code.

You should also learn to not use jQuery. It has had its day and will just set you back as it shelters you from learning modern JS and DOM interfaces.

"use strict";
lol.focus();
lol.addEventListener("keyup", event => {
 const data = lol.value.split("\n");
 const result = data.unique();
 info.textContent = result.length !== data.length ? "Duplicate removed" : "";
 lol.value = result.join('\n'); 
});
if (!Array.prototype.unique) {
 Object.defineProperty(Array.prototype, "unique", {
 configurable: false, // these 3 properties default to false so not needed
 enumerable: false, // But have added them just to show their availability.
 writable: false, 
 value: function() { 
 const existing = {};
 return this.filter(a => existing[a] = !existing[a] ? true : false);
 }
 });
} else {
 throw new Error("Array.prototype.unique already defined.");
}
<textarea id="lol"></textarea>
<div id="info" style="color:red"></div>

answered May 21, 2019 at 14:19
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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.