Welcome to the Treehouse Community
Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.
Looking to learn something new?
Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.
Start your free trialSamuel Cleophas
12,348 PointsI 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" )
7 Answers
Steven Parker
231,275 PointsThis 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
19,323 PointsHiya 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
3,898 Pointsvar 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
231,275 PointsI 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
3,898 PointsThanks 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
231,275 PointsThe 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. :
GREGORY ASSASIE
3,898 PointsAbsolutely. But learning isn't limited to treehouse code challenges.
GREGORY ASSASIE
3,898 PointsOh 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
231,275 PointsI 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.
GREGORY ASSASIE
3,898 PointsThank you
nico dev
20,364 Pointsnico dev
20,364 PointsGreat 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.