Welcome to the Treehouse Community

The Treehouse Community is a meeting place for developers, designers, and programmers of all backgrounds and skill levels to get support. Collaborate here on code errors or bugs that you need feedback on, or asking for an extra set of eyes on your latest project. Join thousands of Treehouse students and alumni in the community today. (Note: Only Treehouse students can comment or ask questions, but non-students are welcome to browse our conversations.)

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and a supportive community. Start your free trial today.

JavaScript JavaScript Loops, Arrays and Objects Tracking Data Using Objects The Student Record Search Challenge

Samuel Cleophas
Samuel Cleophas
12,348 Points

I need help improving my code!

Hi there!

I'm Samuel. Thanks for your time by the way.

I've completed this challenge and my code works well!

I was wondering if you might be able to find any ways to:

  • Improve my logic / method
  • Make my code look more readable.
  • Explain why people like to create unnesscary variables all the time.

Thanks a mil!! :)

Here is my code.

var students = [
  { name: 'Samuel',
    track: 'Front End Web Development',
    achievements: 57, 
    points: 1445 },

  { name: 'Frederick',
    track: 'Full Stack Javascript Development',
    achievements: 200,
    points: 12000 },

  { name: 'Adora',
    track: 'Data Science',
    achievements: 39,
    points: 1259 },

  { name: 'Simon Tan',
    track: 'Front End',
    achievements: 350,
    points: 234000 },

  { name: 'Madeline Winterbottom',
    track: 'Psychology and Law',
    achievements: 145634,
    points: 759000321 }
];

var HTML = '';
var input;

function print (input) {
  var node = document.getElementById('output')
  node.innerHTML = input;
}

do {
  input = prompt("Search student records. Type a name [Jody], or type [quit] to end.");
  for ( i=0; i < students.length; i++ ) {
    if ( input === students[i].name ) {
      HTML += "<h2>" + students[i].name + "</h2>";
      HTML += "<p>" + students[i].track + "</p>";
      HTML += "<p>" + students[i].achievements + "</p>";
      HTML += "<p>" + students[i].points + "</p>";
      print(HTML);
    }
  }
} while ( input !== "quit" )
nico dev
nico dev
20,360 Points

Great stuff, indeed! :)

In addition to the great suggestions already provided, I would always like to be flexible when receiving input, so a good idea could be to add toLowerCase() (or uppercase if you prefer) to both the input and the name in the conditional statement. Trust me from a UX perspective it may be frustrating to type JODY instead of Jody and receive a message that that student is not in the records.

And it would also be nice another message informing the user when the name typed is not found.

Don't get me wrong, your code is simply great! Just providing some suggestions as you asked for.

7 Answers

Steven Parker
Steven Parker
210,867 Points

This program appears to use appropriate logic and is quite readable, two slight improvements would be to use semicolons consistently after statements and to explicitly declare your loop variable (with "var" or "let").

But I'm not aware of the trend to create unnecessary variables that you mention. Could you provide some examples?

Ari Misha
Ari Misha
19,268 Points

Hiya Samuel! Your code is elegant i gotta say but you could make it a lot better. Consider these points to make it more readable and elegant:

  • Use "let" and "const" keywords for best practices
  • Use arrow functions
  • There are a couple missing semi-colons
  • There are a few places you could do loose equality with "==" than "===" especially in conditionals and loops.

And can explain me about your "Students" variable? Did you define it yourself or is it some kind of json data? (its appears to be js object that you defined on your own )

GREGORY ASSASIE
GREGORY ASSASIE
3,898 Points
var students = [
  { name: 'Samuel',
    track: 'Front End Web Development',
    achievements: 57, 
    points: 1445 },

  { name: 'Frederick',
    track: 'Full Stack Javascript Development',
    achievements: 200,
    points: 12000 },

  { name: 'Adora',
    track: 'Data Science',
    achievements: 39,
    points: 1259 },

  { name: 'Simon Tan',
    track: 'Front End',
    achievements: 350,
    points: 234000 },

  { name: 'Madeline Winterbottom',
    track: 'Psychology and Law',
    achievements: 145634,
    points: 759000321 }
];

var HTML = '';
var input;

/*
you priority when writing functions is to be able to reuse them
so i made the node name a variable
*/
function print (nodeName , input) {
  var node = document.getElementById(nodeName)
  node.innerHTML = input;
}
/*

do {
  input = prompt("Search student records. Type a name [Jody], or type [quit] to end.");
  for ( i=0; i < students.length; i++ ) {
    if ( input === students[i].name ) {
      HTML += "<h2>" + students[i].name + "</h2>";
      HTML += "<p>" + students[i].track + "</p>";
      HTML += "<p>" + students[i].achievements + "</p>";
      HTML += "<p>" + students[i].points + "</p>";
      print(HTML);
    }
  }
} while ( input !== "quit" )

*/

/*
i also replaced the while loop with a resuable function 
that accepts the student data and and them map over it to extract the 
the need fields.

things you may have to look up 
      **template literals 
      ** Array.map()
*/
function compose(studentData){
  const HTML = studentData.map( student => {
    return `
        <div>${student.name}</div>
          <p>${sudent.track}</p>
          <p>${sudent.achievements}</p>
          <p>${sudent.points}</p>
        </div>  
    `
  })
}

const htmlString = compose(students);
print('dom class or id', htmlString);

I hope this helps

Steven Parker
Steven Parker
210,867 Points

I don't think it's always necessary to make a function as generic as possible, and the original print function here already met the important design criterion of "fitness for purpose" so the change made here isn't clearly an improvement.

Also, the comment of 'dom class or id' shown as a psuedo-argument in the call isn't correct. The function code would not work if given a class, it requires an ID.

GREGORY ASSASIE
GREGORY ASSASIE
3,898 Points

Thanks for pointing that out Steven Parker. A quick fix would be to change getElementById to querySelector

Again it was just to give him an idea of how it could be improved and the point here was to to make things as declarative as possible.

Take it easy on me . I am a beginner too ????

Steven Parker
Steven Parker
210,867 Points

The idea itself was sound. And if the change had allowed you to use the function more than one place in the program it would have been a real improvement. Not bad for a beginner. ::wink:

GREGORY ASSASIE
GREGORY ASSASIE
3,898 Points

Absolutely. But learning isn't limited to treehouse code challenges.

GREGORY ASSASIE
GREGORY ASSASIE
3,898 Points

Oh Steven maybe i should also as for this favour, I have been learning to code for about 7 months now. But I have no idea if i write readable code or not. If you could please review my github profile if you have any chance and give me your review i would be extremely happy.

github: https://github.com/Gregjarvez

Steven Parker
Steven Parker
210,867 Points

I did take a quick glance and what I saw seemed well formatted, sprinkled with comments, and no obvious inefficiencies. I'd say stylistically you're doing pretty well.