>> I haven't received the quality of feedbck that I'm looking for..
You might not find it here either.
There are a few issues going on here:
1. There is a structural issue in the way you organize you're code. You
expose two global variables when you don't need to, not that it matters that
much in this case. You are using a simple run-the-code-once scenario, which
I personally think warrants the use of an anonymous object, but an anonymous
function works well. Still, there is no need to initialize your code outside
the function, in fact if everything was a function declaration you could
initialize above all your functions. This is what an anonymous object looks
like:
({
Init :function(){/*run your code*/},
method1:function(){this.method2();},
method2:function(){/*does something*/}
}).Init()
2. Just being nitpicky, you've built a getElementsByClass method, and I'm
not entirely sure what it does, but I see you're checking the individual
className of the elements in the method. Chrome has a built in method for
getting elements by class.
3. Lines 66-81 are painful, and I think you know it. Instead of looping
through every element, try giving all the elements a unique id, or something
like that, so that you can hash the results of one of those arrays and
create an O(n) loop instead of an O(n*n) pattern.
4. Buy this book:
http://www.amazon.com/gp/product/047022780X?ie=UTF8&tag=nczonline-20&linkCode=as2&camp=1789&creative=390957&creativeASIN=047022780X
or this book:
http://www.amazon.com/JavaScript-Good-Parts-Douglas-Crockford/dp/0596517742/ref=sr_1_1?ie=UTF8&qid=1317827412&sr=8-1
there are a lot of gadflies in your code that could be easily fixed, these
two books will set you straight.
There is more I could touch on, but those are the big ones. I'm assuming you
wanted a code review for the pleasure of it, because given the platform you
are coding for your mistakes aren't that bad, because your application isn't
that big. Chrome will be very forgiving of every mistake you made. In the
future, try bringing in some cross-browser work you've done and are proud
of, and there will be much more room for people to criticize, because as it
stands I'm sure your application will work fine, and nobody is going to be
able to suggest anything, from a purely code point of view, that will vastly
improve this app.
All the best.
On Wed, Oct 5, 2011 at 7:21 AM, bittersweetryan <[email protected]>wrote:
> I've been trying quite hard to get more professional with my
> JavaScript development lately. I've been doing a lot of reading (blog
> posts, Crockford's book, and Haverbeke's book). Since reading about
> JavaScript is only part of the puzzle I've written a chrome extension
> to try to put what I've read to use.
>
> Unfortunately I work for a small company and I don't have anybody to
> do a code review to help me see my errors and learn from them. I
> totally agree with Addy Osmani's comments on his blog post "Avoiding
> The Quirks: Lessons From A JavaScript Code Review" on how valuable a
> code review can be. I've tried to post my code on
> codereview.stackexchange.com, however I haven't received the quality
> of feedbck that I'm looking for.
>
> The codebase isn't huge ~300 lines and its up on github -
> https://github.com/bittersweetryan/GitHub-News-Filter. If anybody has
> the time to look at it and give me some advise on what I could be
> doing better I'd really, really appreciate it.
>
> Ryan
>
> --
> 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]
