CODEWITHSUNDEEP
javascript
Yerim Kang
Yerim Kang

Reputation: 311

Can I use if else statement with onclick?

I want to Sort array of objects by string property value. So if I click a button, the values will be sorted. But only the first if statement works. if I change the else if statement into the first if statement, it works but no matter what I do only the first if statement works. What should I change to make all three buttons work? 

var members = [{
  name: "jar",
  join: "2017-2",
  sex: "f",
  age: "29"
}, {
  name: "hrk",
  join: "2017-8",
  sex: "f",
  age: "21"
}, {
  name: "jjk",
  join: "2014-6",
  sex: "m",
  age: "27"
}, {
  name: "jsh",
  join: "2016-5",
  sex: "m",
  age: "37"
}, {
  name: "kks",
  join: "2014-4",
  sex: "f",
  age: "42"
}];
var list = "";


function func() {
  for (var i = 0; i < members.length; i++) {
    var val = document.getElementById('form').ans.value;

    if (members[i].join >= val || members[i].name == val || members[i].sex == val || members[i].age == val) {

      for (var key in members[i]) {
        if (key === "name" || key === "join" || key === "sex") {

          if (document.getElementById("namebtn").onclick) {
            var namesort = function(a, b) {
              if (a.name > b.name) {
                return 1;
              } else if (a.name < b.name) {
                return -1;
              }
              return 0;
            }
            members.sort(namesort);
          } else if (document.getElementById("joinbtn").onclick) {
            var joinsort = function(a, b) {
              if (a.join > b.join) {
                return 1;
              } else if (a.join < b.join) {
                return -1;
              }
              return 0;
            }
            members.sort(joinsort);
          } else if (document.getElementById("joinbtn").onclick) {
            var agesort = function(a, b) {
              if (a.age > b.age) {
                return 1;
              } else if (a.age < b.age) {
                return -1;
              }
              return 0;
            }
            members.sort(agesort);
          }
          br = " ";
        } else {
          br = "<br>";
        }
        list += members[i][key] + br;

      }
    }
  }
  document.getElementById("title").innerHTML = title;
  document.getElementById("who").innerHTML = list;
  return false;
}
<p id="title"></p>
<p id="who"></p>
<form action="#" id="form">
  <input type="text" name="ans" placeholder="year">
  <input type="submit" value="search" id="sub">
</form>
<br>
<input type="submit" onclick="func()" id="namebtn">
<input type="submit" onclick="func()" id="joinbtn">
<input type="submit" onclick="func()" id="agebtn">

Upvotes: 0

Views: 8217

Answers (5)

gurvinder372
gurvinder372

Reputation: 68393

But only the first if statement works. if I change the else if statement into the first if statement

Because onclick value is not falsey and else is unlikely to be executed.

What should I change to make all three buttons work?

You can simplify by passing the property by which sorting should happen, here is a demo of the same

var members = [
    { name: "jar", join: "2017-2", sex: "f", age: "29" },
    { name: "hrk", join: "2017-8", sex: "f", age: "21" },
    { name: "jjk", join: "2014-6", sex: "m", age: "27" },
    { name: "jsh", join: "2016-5", sex: "m", age: "37" },
    { name: "kks", join: "2014-4", sex: "f", age: "42" }
];

var list = "";

function func( prop ) 
{
  sortObjByProp(members, prop);
  list = members.map( function( memberObj ){  
     return Object.keys( memberObj ).map( function( key ){
        return "<span class='cell'>" + memberObj[ key ] + "</span>";
     }).join( "  " );
  }).join( "<br>" );
  
  document.getElementById("who").innerHTML = list;
  return false;
}

function sortObjByProp(obj, prop) 
{
  obj.sort(function(a, b) {
     return a[prop].localeCompare(b[prop]); //will work for numerics like age if the number of digits are same for all ages.
  });
}
.cell
{
   display :inline-block;
   width: 80px;
}
<p id="who"></p>
<form action="#" id="form">
  <input type="text" name="ans" placeholder="year">
  <input type="submit" value="search" id="sub">
</form>
<br>
<input type="submit" onclick="func('name')" id="namebtn" value="sort by name">
<input type="submit" onclick="func('join')" id="joinbtn" value="sort by join">
<input type="submit" onclick="func('age')" id="agebtn" value="sort by age">

Upvotes: 0

Rajesh
Rajesh

Reputation: 24925

As rightly suggested by @nitheesh, you should use event.currentTarget.id instead of onClick.

Few pointers,

Note: These pointers are mere suggestions and are highly dependent on individual's perspective. If you think they help you in anyway, use them. If not, just ignore them.

  • If you define document.getElementById(...).onClick = function(){...}, this will replace your original handler. It might even break the logic
  • Having a handler on markup can make your code more fragile as they are rendered along with HTML, so anyone looking at the source of your page can see the names. You should use addEventListener for this.
  • You should try to reuse your code more. Instead of having multiple sort functions, have a generic function that returns a sort function. This way you can handle more cases with less code.
  • Try to use better naming conventions. func is a bad function name. A function name should denote its purpose.

var members = [{ name: "jar", join: "2017-2", sex: "f", age: "29" }, { name: "hrk", join: "2017-8", sex: "f", age: "21" }, { name: "jjk", join: "2014-6", sex: "m", age: "27" }, { name: "jsh", join: "2016-5", sex: "m", age: "37" }, { name: "kks", join: "2014-4", sex: "f", age: "42" }];

function getMySortFn(key) {
  return function(a, b) {
    return a[key].localeCompare(b[key]);
  }
}

function func(e) {
  var list = "";
  for (var i = 0; i < members.length; i++) {
    var val = document.getElementById('form').ans.value;

    if (members[i].join >= val || members[i].name == val || members[i].sex == val || members[i].age == val) {
    var shouldSort = ['namebtn', 'joinbtn', 'agebtn'].indexOf(e.currentTarget.id) > -1;
      for (var key in members[i]) {
        if (key === "name" || key === "join" || key === "sex") {
          if(shouldSort) {
            members.sort(getMySortFn(e.currentTarget.id.replace('btn', '')));
          }
          br = " ";
        } else {
          br = "<br>";
        }
        list += members[i][key] + br;
      }
    }
  }
  document.getElementById("title").innerHTML = title.innerHTML;
  document.getElementById("who").innerHTML = list;
  return false;
}

var btns = document.querySelectorAll('input[type="submit"]');
for (var i = 0; i < btns.length; i++) {
  btns[i].addEventListener('click', func);
}
<p id="title"></p>
<p id="who"></p>
<form action="#" id="form">
  <input type="text" name="ans" placeholder="year">
  <input type="submit" value="search" id="sub">
</form>
<br>
<input type="submit" id="namebtn" value='name'>
<input type="submit" id="joinbtn" value='join'>
<input type="submit" id="agebtn" value='age'>

Upvotes: 0

guest271314
guest271314

Reputation: 1

You can separate the logic by defining the sort functions outside of func function, attach event handlers to each element using .addEventListener() or .onclick. Call func() within an event handler attached to the element. Use var to define variables, specifically br and list

var members = [{
  name: "jar",
  join: "2017-2",
  sex: "f",
  age: "29"
}, {
  name: "hrk",
  join: "2017-8",
  sex: "f",
  age: "21"
}, {
  name: "jjk",
  join: "2014-6",
  sex: "m",
  age: "27"
}, {
  name: "jsh",
  join: "2016-5",
  sex: "m",
  age: "37"
}, {
  name: "kks",
  join: "2014-4",
  sex: "f",
  age: "42"
}];

var namesort = function(a, b) {
  if (a.name > b.name) {
    return 1;
  } else if (a.name < b.name) {
    return -1;
  }
  return 0;
}

var joinsort = function(a, b) {
  if (a.join > b.join) {
    return 1;
  } else if (a.join < b.join) {
    return -1;
  }
  return 0;
}

var agesort = function(a, b) {
  if (a.age > b.age) {
    return 1;
  } else if (a.age < b.age) {
    return -1;
  }
  return 0;
}

document.getElementById("joinbtn").onclick = function() {
  members.sort(joinsort);
  func();
};

document.getElementById("agebtn").onclick = function() {
  members.sort(agesort);
  func();
}

document.getElementById("namebtn").onclick = function() {
  members.sort(namesort);
  func();
};

function func() {
  var br = "";
  var list = "";
  for (var i = 0; i < members.length; i++) {
    var val = document.getElementById('form').ans.value;

    if (members[i].join >= val || members[i].name == val || members[i].sex == val || members[i].age == val) {

      for (var key in members[i]) {
        if (key === "name" || key === "join" || key === "sex") {
          br = " ";
        } else {
          br = "<br>";
        }
        list += members[i][key] + br;

      }
    }
  }
  // document.getElementById("title").innerHTML = title;
  document.getElementById("who").innerHTML = list;
  return false;
}
<p id="title"></p>
<p id="who"></p>
<form id="form">
  <input type="text" name="ans" placeholder="year">
  <input type="button" value="search" id="sub">
</form>
<br>
<input type="button" id="namebtn" value="name">
<input type="button" id="joinbtn" value="join">
<input type="button" id="agebtn" value="age">

Upvotes: 0

Nitheesh
Nitheesh

Reputation: 19986

Rather than checking document.getElementById("elementid").onclick, you can find the target of the current click event. Look this example.

var members = [{name: "jar",join: "2017-2",sex: "f",age: "29"}, 
               {name: "hrk",join: "2017-8",sex: "f",age: "21"},
               {name: "jjk",join: "2014-6",sex: "m",age: "27"},
               {name: "jsh",join: "2016-5",sex: "m",age: "37"},
               {name: "kks",join: "2014-4",sex: "f",age: "42"}];
var list = "";


function func(e) {
    for (var i = 0; i < members.length; i++) {
        var val = document.getElementById('form').ans.value;
        if (members[i].join >= val || members[i].name == val || members[i].sex == val || members[i].age == val) {

            for (var key in members[i]) {
                if (key === "name" || key === "join" || key === "sex") {

                    if (e.currentTarget.id == "namebtn") {
                        var namesort = function (a, b) {
                            if (a.name > b.name) {
                                return 1;
                            } else if (a.name < b.name) {
                                return -1;
                            }
                            return 0;
                        }
                        members.sort(namesort);
                    } else if (e.currentTarget.id == "joinbtn") {
                        var joinsort = function (a, b) {
                            if ((new Date(a.join)).getTime() > (new Date(b.join)).getTime()) {
                                return 1;
                            } else if ((new Date(a.join)).getTime() < (new Date(b.join)).getTime()) {
                                return -1;
                            }
                            return 0;
                        }
                        members.sort(joinsort);
                    } else if (e.currentTarget.id == "agebtn") {
                        var agesort = function (a, b) {
                            if (parseInt(a.age) > parseInt(b.age)) {
                                return 1;
                            } else if (parseInt(a.age) < parseInt(b.age)) {
                                return -1;
                            }
                            return 0;
                        }
                        members.sort(agesort);
                    }
                    br = " ";
                } else {
                    br = "<br>";
                }
                list += members[i][key] + br;

            }
        }
    }
    document.getElementById("title").innerHTML = title;
    document.getElementById("who").innerHTML = list;
    return false;
}
<p id="title"></p>
<p id="who"></p>
<form action="#" id="form">
    <input type="text" name="ans" placeholder="year">
    <input type="submit" value="search" id="sub">
</form>
<br>
<input type="submit" onclick="func(event)" value="name" id="namebtn">
<input type="submit" onclick="func(event)" value="join" id="joinbtn">
<input type="submit" onclick="func(event)" value="age" id="agebtn">

Also you can use some better comparison methods for date and age. I have updated that in my code.

Upvotes: 0

dork
dork

Reputation: 4568

if (document.getElementById('namebtn').onclick) will always be true since the attribute is truthy.

Are you trying to check which button was clicked? If that's the case, your func should probably have an id or event parameter (function func(btnId or e) { ... }) and then pass this.id to func:

<input type="submit" onclick="func(this.id or event)" id="namebtn">

Upvotes: 1

Related Questions