dafie
dafie

Reputation: 1169

Order elements by attribute

I have a list of items and two arrays:

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

Answers (3)

Nikhil Aggarwal
Nikhil Aggarwal

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.
  • If the order is the same as required, then return -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.
  • When 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)
  • There is an unused function isNotMarked which should be removed

You 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

Eduardo Waghabi
Eduardo Waghabi

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

Louis
Louis

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

Related Questions