Suggestions on how I can improve my code.

Question

My Solution

var message = '',
    student,
    studentIndex,
    search;

function print(message) {
  var outputDiv = document.getElementById('output');
  outputDiv.innerHTML = message;
}

function getIndex(arrayName, search, value) {
    for(var i = 0; i < arrayName.length; i++) {
        if (arrayName[i][value].toLowerCase() === search) return i;
    }
    return -1;
}

while(true){
  search = prompt("Search student records by typing a name for eg. 'Jody', or you can quit by typing 'quit'");
  search = search.toLowerCase();
  studentIndex = getIndex(students, search, "name");
  console.log(studentIndex);
  if(studentIndex != -1){
    student = students[studentIndex];
    console.log(student);
    name = student.name.toLowerCase();
    console.log(name);
    if(name === search){
      message = '<h2>Student: ' + student.name + '</h2>';
      message += '<p>Track: ' + student.track + '</p>';
      message += '<p>Points: ' + student.points + '</p>';
      message += '<p>Achievements: ' + student.achievements + '</p>';
      print(message);
    }
  }else if(search === "quit"){
      message += '<h2>You quit the search.</h2>';
      print(message);
      break;
  }else if(studentIndex == -1){
    message = '<h2>Name does not exist!</h2>';
    print(message);
  }
}

Answer 1 · 2015-09-03T04:11:10Z

September 3, 2015 4:11am

I like your getIndex function, since it's like an indexOf for checking the property in objects in an array.

Some suggestions/comments:

You should check for the (search === "quit") first, since then the program doesn't have to go through the entire students array.

There's no point checking if(name === search) since you already know within that part of the conditional that it matches an existing student.

Also, you don't need an else if at the end, an else would do.

Here's mine:

var html = '';
var query;


function print(nodeID, message) {
  document.getElementById(nodeID).innerHTML = message;
}

function buildDefinitionList(obj) {
  var prop;
  var dListHTML = '<dl>';
  for (prop in obj) {
    dListHTML += '<dt>' + prop.charAt(0).toUpperCase() + prop.slice(1) + '</dt>';
    dListHTML += '<dd>' + obj[prop] + '</dd>';
  }
  dListHTML += '</dl>';
  return dListHTML;
}

function buildStudentList(list) {
  var i;
  var uListHTML = '<ul>';
  for (i = 0; i < list.length; i++) {
    uListHTML += '<li>' + buildDefinitionList(list[i]) + '</li>';
  }
  uListHTML += '</ul>';
  return uListHTML;
}

function getMatchingStudents(list) {
  var i;
  var resultList;
  // keep looping until the user quits
  while(true) {
    // reset the result list
    resultList = [];
    // prompt and convert to lowercase
    query = prompt("Search student records: type a name [Iain] (or type 'quit' to end)").toLowerCase();
    // check if they want to quit
    if (query === 'quit') {
      print('output', '<p>Thanks! Bye!</p>');
      break;
    }
    // loop through students and check if the name contains the search query
    for (i = 0; i < list.length; i++) {
      if (list[i].name.toLowerCase().indexOf(query) > -1) {
        // if so, push to the result list
        resultList.push(list[i]);
      }
    }    
    // if there are results
    if (resultList.length > 0) {
      // build the list of matching students
      html = buildStudentList(resultList);
    } else {
      // otherwise print a friendly not found message
      html = '<p>Sorry, there are no students matching that name</p>';
    }    
    // print html to output div
    print('output', html);
  }
}

getMatchingStudents(students);

Welcome to the Treehouse Community

Looking to learn something new?

Sakib Rahman

Sakib Rahman

Suggestions on how I can improve my code.

1 Answer

Iain Simmons

Iain Simmons