Why does this piece of code always return "no_conflict"?

Question

I wrote a method as follows:

detectNameConflict: function() {
    var existing_filenames = this.element.find('ul.existing_files > li');
    if (existing_filenames.length > 0) {
        var try_name = this.element.find('div.target_filename').text().trim();
        existing_filenames.each(function(index, el) {
            if ($(el).text() == try_name) {
                return "contain_conflict";
            }
        });
    } else {
      return "no_conflict";
    }
},

This code doesn't work, because it always returns "no_conflict", even when there is a naming conflict.

note: this.element is from jQueryUI widget factory. It refers to the DOM element on which the widget instance attached.

By the way, is this multiple return point pattern considered bad practice in Javascript? — Jinghui Niu, Nov 14 '16 at 22:35
Multiple return points are perfectly acceptable, but it doesn't work from inside nested functions. Your `"contain_conflict"` is not returning from the outer function, it's returning from the inner function you created. — 4castle, Nov 14 '16 at 22:38
The only thing `.each()` uses the return value for is deciding whether to keep looping. If you return `false` the loop stops, otherwise it keeps going. — Barmar, Nov 14 '16 at 22:39
The only time it returns `no_conflict` is when `existing_filenames` is empty. When `length > 0`, it returns `undefined`. — Barmar, Nov 14 '16 at 22:43
console.log() is your friend, see why the check is never valid. Seems like `var existing_filenames = this.element.find('ul.existing_files > li');` is returning 0 items — epascarello, Nov 14 '16 at 22:58

score 1 · Accepted Answer · answered Nov 14 '16 at 22:47

1

You can convert the jQuery collection into an array, then use the Javascript some() method to test if any of them match try_name.

detectNameConflict: function() {
    var try_name = this.element.find('div.target_filename').text().trim();
    var existing_filenames = this.element.find('ul.existing_files > li').toArray();
    if (existing_filenames.some(function(el) {
        return $(el).text() == try_name;
    })) {
        return "contain_conflict";
    } else {
        return "no_conflict";
    }
}

answered Nov 14 '16 at 22:47

Barmar

741,623
53
500
612

Is this "exiting predicate instead of the procedure" stuff only in Javascript? I've never seen such problem in Python. Although I admit that I'm a beginner for both languages. Just want to clarify this piece of knowledge. Thank you. – Jinghui Niu Nov 15 '16 at 09:51
In Python, if you call a lambda from another function, and the lambda has `return`, it will just return from the lambda, not the calling function. – Barmar Nov 15 '16 at 17:01

Jacob Manaker · Answer 2 · 2016-11-14T23:05:42.347

Your return statement for "contain_conflict" exits the predicate, not the procedure as a whole, so each continues to loop after a conflict is detected. You need to either use a for loop or exception-based control flow (seeing as JavaScript lacks goto).

Update

Barman has pointed out in the comments above that you can stop the looping by returning false from your predicate. Prefer that to exceptions; I wasn't aware of that functionality when I wrote this answer.

End Update

A for loop:

detectNameConflict: function()
{
    var existing_filenames = this.element.find('ul.existing_files > li');
    if (existing_filenames.length > 0)
    {
        var try_name = this.element.find('div.target_filename').text().trim();
        for(var filename in existing_filenames)
        {
            if ($(filename).text() == try_name)
            {
                return "contain_conflict";
            }
         }
    }
    return "no_conflict";
}

Exception-based flow:

detectNameConflict: function()
{
    var existing_filenames = this.element.find('ul.existing_files > li'); 
    if (existing_filenames.length > 0)
    {
        var try_name = this.element.find('div.target_filename').text().trim();
        try
        {
            existing_filenames.each(function(index, el)
                {
                    if ($(el).text() == try_name)
                    {
                        throw "contain_conflict";
                    }
                });
        }
        catch(e)
        {
            return e;
        }
    }
    return "no_conflict";
}

*because it always returns "no_conflict"* - means it is always in the else... means the if statement is always false — epascarello, Nov 14 '16 at 23:03
[Don't use `for…in` enumerations on array-like objects!](http://stackoverflow.com/q/500504/1048572) — Bergi, Nov 14 '16 at 23:23

Why does this piece of code always return "no_conflict"?

2 Answers2