Checking for the presence of an element in the array does not work

Question

Sorry to bother you, but I've been figuring it out for two hours. The problem is that my code adds only one element, but does not add other ids, writes "already have"

$(function(){

    $(".js-compare-add").click(function(){
        var item_id = $(this).data('id');

        if(Cookies.get('compare_id_list')){
            var compareListStr = Cookies.get('compare_id_list');
            var compareListArr = compareListStr ? compareListStr.split(',') : [];
            if (compareListArr.indexOf(item_id) > -1) {
                compareListArr.push(item_id);
                Cookies.set('compare_id_list', compareListArr.join(','), {expires: 10}); 
                alert('Save! ' + item_id);
            } else {
                alert('already have ' + item_id);
            }

        } else {
            alert('There was no list, we created it and added this position ' + item_id);
            Cookies.set('compare_id_list', item_id);
        }

    });
});

Just in case - I used it for cookies https://github.com/js-cookie/js-cookie if (jQuery.inArray(item_id, compareListArr) === -1) didn't help either

If you find an index > 1 you push the item—if it’s > -1 doesn’t that mean it exists? — Dave Newton, Mar 22 '23 at 18:15

T.J. Crowder · Accepted Answer · 2023-03-22T18:27:04.253

Dave Newton raises a useful point that the logic seems backward, but in case that's the result of your playing with it to try to get it to work:

jQuery does data manipulation when you use the data function, it doesn't just give you the string value of the matching data-id attribute. If the text of the attribute is all digits, jQuery will convert it to number:

const id = $("#example").data("id");
console.log(typeof id); // number

<div id="example" data-id="123"></div>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

So indexOf won't find it in an array of strings, since it does an === comparison.

If the ID should be a string and you don't have some particular reason for using data, use attr instead:

const item_id = $(this).attr("data-id");

In my experience, the vast majority of times people use data("x"), they really wanted attr("data-x") instead. data is not just an accessor for data-* attribuets, it's both more and less than that.

Alternatively, if you want the ID to be a number, go the other way: Convert the elements of your array to number after splitting:

const compareListArr = compareListStr
    ? compareListStr.split(",").map((id) => +id)
    : [];

(The unary + is just one option for parsing numbers, see others here.)

Side note: In new code, I'd prefer includes over indexOf. Here's that if with the logic corrected and using includes (note that I've switched the if and else blocks):

if (compareListArr.includes(item_id)) {
    // Exists, don't push it
} else {
    // Doesn't exist, push it
}

Thank you very much, you helped me a lot! Sorry for my inattention to data types! — Lememlol, Mar 22 '23 at 18:27
@Lememlol - :-) It can be really hard to see the difference between `42` and `"42"` when debugging, particularly if debugging via `console.log`. (I recommend using a debugger instead, partially for that reason.) Happy coding! — T.J. Crowder, Mar 22 '23 at 18:28

Checking for the presence of an element in the array does not work

1 Answers1