Bubble,
I do have some critiques about your JavaScript file. I am not a jQuery fan, so
I will not comment on such conventions or logic.
1) I see that you are using named and unassigned function in some cases and
anonymous functions as arguments in other cases. I would pick a single
consistent implementation and stick with it as this will help you to maintain
your application as it grows into the future.
2) I have some concern where you are testing for the presence of the "storage"
object. I would recommend changing:
if (storage) {
To
if (typeof localeStorage === 'object') {
The reason for this is that you appear to only be using localStorage and not
globalStorage or the other variations. Be specific so as to avoid uncertainty
in potential edge cases. Testing against typeof is generally considered the
safest way to validate whether or nothing is undefined, which is important
cross browser.
3) You are declaring a "loadData" function before you are testing for the
presence of the storage, or localStorage, object. If localStorage were not
available this would throw and error.
I would be willing to provide more extensive feedback if you were not using
jQuery.
Thanks,
Austin Cheney, CISSP
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf
Of cancel bubble
Sent: Wednesday, October 26, 2011 9:05 AM
To: The JSMentors JavaScript Discussion Group
Subject: [JSMentors] I "redesigned" Hacker News - would love a JS code review
The "redesign": http://www.cancelbubble.com/hackernews/
I made a quick screencast (link top left) walking through the
features.
Hacker News thread just posted: http://news.ycombinator.com/item?id=3158343
Might have a little more context, but the screencast pretty much
covers it as well.
The JavaScript file: http://www.cancelbubble.com/hackernews/javascript/global.js
I do mix spaces and tabs in my IDE (oh the horror) so this output,
while not horrible, is not perfect (looks fine in the IDE).
I also posted up a jsFiddle where I cleaned up some of the spacing
issues: http://jsfiddle.net/wPwbF/
If you have suggestions for improvements, you could make them here so
I can see what you're talking about and what you did.
Thanks much!
--
To view archived discussions from the original JSMentors Mailman list:
http://www.mail-archive.com/[email protected]/
To search via a non-Google archive, visit here:
http://www.mail-archive.com/[email protected]/
To unsubscribe from this group, send email to
[email protected]
--
To view archived discussions from the original JSMentors Mailman list:
http://www.mail-archive.com/[email protected]/
To search via a non-Google archive, visit here:
http://www.mail-archive.com/[email protected]/
To unsubscribe from this group, send email to
[email protected]
