Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was hoping somebody could help me refactor my Javascript. Due to it's length (500 or so lines) I don't want to post it here. But check it out here on pastebin to let me know how I can improve it please! It's for a multi-step form.

Javascript: http://pastebin.com/eUVpC2wi

HTML Page: http://pastebin.com/Dv6QbmCN

Thank you so much for your help!

share|improve this question
2  
You should include the code in your post per FAQ – mseancole Dec 20 '12 at 22:44
agreed , you should include the code in your post , if it too large , break it section you want to improve. – paritosh Dec 21 '12 at 3:42
Sorry about that. Honestly, I'm new enough at Javascript that I don't know what to show and what not to for this. I don't know what needs to be refactored and what can't be refactored. Sorry, hence why I'm here. To try to learn and get some people to help me learn with some reviews. – kkirsche Dec 21 '12 at 14:42

closed as off topic by Winston Ewert Dec 27 '12 at 1:37

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined in the FAQ. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about closed questions here.

2 Answers

up vote 0 down vote accepted

Here is one way to get rid of a lot of redundancy in your code.

$('div.ifScholarshipsSelected input[type="checkbox"]').click(function () {
    $('#' + $(this).val().replace(/_/g,'') + 'Allocation').slideToggle("fast");
});

That code would replace all of these:

$("#showClassof1954Allocation").click(function () {
    if ($("#showClassof1954Allocation:checked").val() === "Class_Of_1954") {
        $("#classOf1954Allocation").slideToggle("fast");
    } else {
        $("#classOf1954Allocation").slideToggle("fast");
    }
});
share|improve this answer
Thank you for your help! What is the .replace(/_/g, '') doing? – kkirsche Dec 21 '12 at 14:44
Got it to work! Thank you! That's great! What's the g in the replace function for though? I know the /_/ but not the g. Thank you again! – kkirsche Dec 21 '12 at 15:21
@kkirsche: The g modifier indicates that the replacement applies to all instances, rather than only applying to the first match. – Brian Dec 26 '12 at 22:43

Think about the TypeScript , a plateform independent object oriented implementation from Microsoft, still it is in development (i mean no production roll out as far as i know ) but worth give it a try. Type Script

share|improve this answer
I'll check it out to see. Thank you! – kkirsche Dec 21 '12 at 14:43

Not the answer you're looking for? Browse other questions tagged or ask your own question.