5
\$\begingroup\$

Is there a better way to do this?

 var duplicate = false;
 for (var i = 0; i < acceptedFiles.length; i++) {
 for (var j = 0; j < attachments.attachments.length; j++) {
 if (attachments.attachments[j].name === acceptedFiles[i].name) {
 duplicate = true;
 }
 }
 }
asked Nov 12, 2014 at 15:30
\$\endgroup\$
1
  • 3
    \$\begingroup\$ You should keep a look out for Set this would allow you to build a set of your acceptedFiles and pass that around. Thus you could remove the outer for loop and simply query the set if the attachment is accepted. If you have many accepted types, this could also speed up significantly as you go from \$\mathcal{O}(nk)\$ to \$\mathcal{O}(n)\$. \$\endgroup\$ Commented Nov 12, 2014 at 15:48

4 Answers 4

8
\$\begingroup\$

Presumably, once a duplicate is found you don't need to continue looping:

if (attachments.attachments[j].name === acceptedFiles[i].name) {
 duplicate = true;
}

With JavaScript, the best thing would be to wrap up your for loops in a function:

function containsDuplicate(acceptedFiles, attachments) {
 for (var i = 0; i < acceptedFiles.length; i++) {
 for (var j = 0; j < attachments.attachments.length; j++) {
 if (attachments.attachments[j].name === acceptedFiles[i].name) {
 return true;
 }
 }
 }
 return false;
}

As this is a problem that applies to multiple languages, look at the break keyword.

answered Nov 12, 2014 at 15:38
\$\endgroup\$
3
  • \$\begingroup\$ @anorton I can't quite wrap my head around how that works here. When i = 1 we are missing the comparison of x[1] to y[0]? \$\endgroup\$ Commented Nov 12, 2014 at 17:18
  • \$\begingroup\$ Oops. I read attachments and acceptedFiles as the same array. If we were looking for duplicates in the same array, it works. sorry. \$\endgroup\$ Commented Nov 12, 2014 at 17:20
  • \$\begingroup\$ @anorton Ahhhhhh that makes sense now \$\endgroup\$ Commented Nov 12, 2014 at 17:21
2
\$\begingroup\$

Using ES5 you can generally avoid having to do tedious for loops to iterate through the values. In this case you can use the some method on arrays in order avoid using a flag and having i and j iterators to get the array values. You also get the added bonus of some short circuiting and stopping when the first true is found so as far as big O notation it is more efficient.

function hasDuplicate() {
 return acceptedFiles.some(function(acceptedValue) {
 return attachments.attachments.some(function(attachmentValue) {
 return acceptedValue.name === attachmentValue.name;
 });
 });
}

If your browser doesn't support ES5 you can use a shim

answered Nov 12, 2014 at 21:55
\$\endgroup\$
2
\$\begingroup\$

If using external libraries is an option, then LoDash would allow for something more like:

var duplicate = _.intersection(_.pluck(acceptedFiles, 'name'),
 _.pluck(attachments, 'name')).length > 0;

or:

function areDuplicates(acceptedFiles, attachments) {
 var acceptedFileNames = _.pluck(acceptedFiles, 'name');
 var attachmentNames = _.pluck(attachments, 'name');
 return _.intersection(acceptedFileNames, attachmentNames).length > 0;
}
answered Nov 12, 2014 at 15:45
\$\endgroup\$
1
  • \$\begingroup\$ I like this (and it's more concise so +1), but in terms of performance, it's probably less efficient than the OP's existing code \$\endgroup\$ Commented Nov 12, 2014 at 16:32
1
\$\begingroup\$

You can return as soon as a match is found.

for (var i = 0; i < acceptedFiles.length; i++) {
 for (var j = 0; j < attachments.attachments.length; j++) {
 if (attachments.attachments[j].name === acceptedFiles[i].name) {
 return true;
 }
 }
}
RubberDuck
31.2k6 gold badges73 silver badges176 bronze badges
answered May 14, 2015 at 12:03
\$\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.