Reputation: 1169
I have a list of items and two arrays:
favouriteItems
blockedItems
I want to reorder items by:
The problem with my script is that "blocked" items are above "not marked" ones.
What's wrong there?
var favouriteItems = [2];
var blockedItems = [1, 3];
function isFavourite(id) {
return (favouriteItems.includes(id));
}
function isBlocked(id) {
return (blockedItems.includes(id));
}
function isNotMarked(id) {
return ((!isFavourite(id) && (!isBlocked(id))));
}
$("div.container div.item").sort(function(a, b) {
var aId = $(a).data("id");
var bId = $(b).data("id");
// check favourites
if ((isFavourite(aId)) && (!isFavourite(bId)))
return 1;
if ((isFavourite(bId)) && (!isFavourite(aId)))
return -1;
if ((isFavourite(aId)) === (isFavourite(bId)))
return 0;
// check blocked
if ((isBlocked(aId)) && (!isBlocked(bId)))
return -1;
if ((isBlocked(bId)) && (!isBlocked(aId)))
return 1;
if ((isBlocked(aId)) === (isBlocked(bId)))
return 0;
// both are not marked
return 0;
}).appendTo("div.container");
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="container">
<div class="item" data-id=1>Banana B</div>
<div class="item" data-id=2>Carrot F</div>
<div class="item" data-id=3>Orange B</div>
<div class="item" data-id=4>Apple</div>
<div class="item" data-id=5>Carrot</div>
</div>
Upvotes: 1
Views: 76
Reputation: 28455
Here is the list of problems and improvements required in the code
check blocked
section and subsequent return
was never executed because the function returned before that only as it covered all the 4 cases of equality between 2 variables.-1
and to swap return 1
. Hence, when a
is favorite and b
is not, return -1. Similarly, when a
is blocked and b
is not, return 1.a
and b
are either both favorites or both not favorites, let the code check for being a
and b
are blocked or not and then finally return 0
(no change)isNotMarked
which should be removedYou can simplify your sort logic as following.
var favouriteItems = [2];
var blockedItems = [1, 3];
function isFavourite(id) {
return (favouriteItems.includes(id));
}
function isBlocked(id) {
return (blockedItems.includes(id));
}
$("div.container div.item").sort(function(a, b) {
var aId = $(a).data("id");
var bId = $(b).data("id");
return isFavourite(bId) - isFavourite(aId) || isBlocked(aId) - isBlocked(bId);
}).appendTo("div.container");
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="container">
<div class="item" data-id=1>Banana B</div>
<div class="item" data-id=2>Carrot F</div>
<div class="item" data-id=3>Orange B</div>
<div class="item" data-id=4>Apple</div>
<div class="item" data-id=5>Carrot</div>
</div>
Also, you can correct the logic in the same coding style as follows.
var favouriteItems = [2];
var blockedItems = [1, 3];
function isFavourite(id) {
return (favouriteItems.includes(id));
}
function isBlocked(id) {
return (blockedItems.includes(id));
}
$("div.container div.item").sort(function(a, b) {
var aId = $(a).data("id");
var bId = $(b).data("id");
// check favourites
if ((isFavourite(aId)) && (!isFavourite(bId)))
return -1;
if ((isFavourite(bId)) && (!isFavourite(aId)))
return 1;
// check blocked
if ((isBlocked(aId)) && (!isBlocked(bId)))
return 1;
if ((isBlocked(bId)) && (!isBlocked(aId)))
return 0;
// both are not marked
return 0;
}).appendTo("div.container");
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="container">
<div class="item" data-id=1>Banana B</div>
<div class="item" data-id=2>Carrot F</div>
<div class="item" data-id=3>Orange B</div>
<div class="item" data-id=4>Apple</div>
<div class="item" data-id=5>Carrot</div>
</div>
For reference, Array.sort
Upvotes: 4
Reputation: 41
The way your sort function is written, the code after "check block" will never be executed because the function will necessarily end at one of the three first returns.
If you write a single function for attributing value to the items that takes all the criteria in consideration, and then use it to sort them, It will work:
var favouriteItems = [2];
var blockedItems = [1, 3];
function isFavourite(id) {
return (favouriteItems.includes(id));
}
function isBlocked(id) {
return (blockedItems.includes(id));
}
function isNotMarked(id) {
return ((!isFavourite(id) && (!isBlocked(id))));
}
function sortValue(id) {
var sortValue = 0;
if (isFavourite(id)) sortValue += 2;
if (isNotMarked(id)) sortValue += 1;
return sortValue;
}
$("div.container div.item").sort(function(a, b) {
var aId = $(a).data("id");
var bId = $(b).data("id");
return sortValue(bId) - sortValue(aId);
}).appendTo("div.container");
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="container">
<div class="item" data-id=1>Banana B</div>
<div class="item" data-id=2>Carrot F</div>
<div class="item" data-id=3>Orange B</div>
<div class="item" data-id=4>Apple</div>
<div class="item" data-id=5>Carrot</div>
</div>
Upvotes: 1
Reputation: 191
It looks to me like you are sorting your items the wrong way around!
The sorting function f(a,b)
should return 1 if b
comes before a
.
In your solution the exact opposite happens
if ((isFavourite(aId)) && (!isFavourite(bId)))
return 1;
If a
is a favourite and b
is not, then a
should come before b
. Yet returning 1 indicates b
becomes before a
.
See docs
Upvotes: 1