Nathan, your feedback has already surpassed what I've received. Thank you!
On Oct 5, 10:18 am, Nathan Sweet <[email protected]> wrote: > >> 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&;... > or this > book:http://www.amazon.com/JavaScript-Good-Parts-Douglas-Crockford/dp/0596... > 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]
