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

I've been coding for a while, but this is the first actual project I've made. I generally do contest programming, and as you might imagine, good code practices are not exactly the top priority. I'd appreciate any pointers, whether overall design of the program, or variable naming, or things that I should have split up into more functions.

Note: This program crawls the HTML of a Schoolloop page, retrieves whether it's a percentage based final or points based, and outputs the grade on the final required to get a specific grade.

function httpGet(theUrl){             //gets raw_html code of page. Credit to Stackoverflow user Joan
    var xmlHttp = null;
    xmlHttp = new XMLHttpRequest();
    xmlHttp.open( "GET", theUrl, false );
    xmlHttp.send( null );
    return xmlHttp.responseText;
}

//#########################################################################################################################
var raw_raw_html=String(httpGet(document.URL));
var value_final_percent, needed_grade, current_grade, name, period, id;       //value_final_percent=percentage of final, needed_grade=needed grade, current_grade=current grade, id=name+period
var max_in_category, ccat, tpoints, cpoints;        //max_in_category=total in category, ccat=current points in category
var final_points;
var start;
var is_weighted;
max_in_category={};
ccat={};
tpoints=0;
cpoints=0;
$("<div/>", {id: "needed_grades"}).appendTo("#page_title");
$("<div/>", {id: "popup", style:visibility="hidden"}).appendTo("#container_page");
//This line is the raw_html for the popup. Probably a better solution, but eh.
popup.innerHTML+=('<form id="user_final" method="GET">'                 +
                    '<input type="text" name="inputbox">'               +
                    '</form> '                                          +   
                    ''                                                  +
                    '<select id="type_final"> '                         +
                    '<option value="percent">Percentage</option>'       +
                    '<option value="points" selected>Points</option>'   +
                    '</select>');

var needed_grade=document.getElementById("needed_grades");
needed_grade.style.fontSize = "25px";
//##########################################################################################################################

//this prevents the default action(going to another page)
//and closes the popup.
$('#popup').keydown(function(e) {
        if (e.keyCode == $.ui.keyCode.ENTER){
            update();
            $("#popup").dialog("destroy");
        }
});

function exist(int index){
    return index==-1 ? false : true;
}   

function find_values(){                                                          //Finds the value of value_final_percent, name, current_grade
    start=raw_html.indexOf("<td class='list_label' nowrap>Weight:</td>");    //The marker used if the class is percentage based. except for Mr. Daniel
    if(start!=-1){
        is_weighted=true;
    } else{
        is_weighted=false;
    }   
    console.log(start);
    if(!is_weighted){
        var limit=raw_html.indexOf("Grade Trend");
        console.log("Grade Trend", limit);
        var cur=0;
        while(cur < limit && cur!=-1 && t!=-1 && score!=-1 && begin!=-1){
            var t=raw_html.indexOf('style="width: 265px;"', cur+1);  //finds the category it belongs in.
            if(t==-1) break;
            var begin=raw_html.indexOf(">", t);
            var end=raw_html.indexOf("<br", begin);
            var cat=raw_html.substring(begin+1, end);
            var score=raw_html.indexOf("Score:", end);                 //finds points achieved on assignment.
            var check=raw_html.indexOf('<b class="red">', score);

            if(check-score<30&&check!=-1){                                      //workaround for zeros.
                begin=raw_html.indexOf(">", check);
                end=raw_html.indexOf("</b>", begin);
            }   
            else{
                begin=raw_html.indexOf(" ", score);
                end=raw_html.indexOf("</div", begin);
            }

            var gotscore=raw_html.substring(begin+1, end);

            begin=Math.min(raw_html.indexOf("/ ", end), raw_html.indexOf("-", end)); //finds max points on assignment
            if(raw_html[begin]=='-'){                           //workaround for extra credit.
                outof="0";
                cur=begin;
            }
            else{
                end=raw_html.indexOf(" =", begin);
                outof=raw_html.substring(begin+2, end);
            }

            cur=end;
            if(cat in max_in_category){
                max_in_category[cat]=parseInt(max_in_category[cat])+parseInt(outof);
                ccat[cat]=parseInt(ccat[cat])+parseInt(gotscore);
            } else{
                max_in_category[cat]=outof;
                ccat[cat]=gotscore;
            }
            tpoints+=parseInt(outof);
            cpoints+=parseInt(gotscore);
            /*console.log(cat);
            console.log(ccat[cat]);
            console.log(max_in_category[cat]);
            console.log("#1:"+gotscore);
            console.log("#2:"+outof);*/
        }
    }
    else if(!is_weighted){                                                           //If this is a percentage based final
        start=raw_html.toLowerCase().indexOf("final", start);
        //console.log(start);
        if(!is_weighted) {
            value_final_percent=raw_html.substring(raw_html.toLowerCase().indexOf("wrap>", start)+5, raw_html.toLowerCase().indexOf("%", start));
        }
        var t=raw_html.indexOf("Score:");
        current_grade=raw_html.substring(t+17, raw_html.indexOf("%", t));
    }

    var t=raw_html.indexOf('"logged_in"');     //finds name of user
    var begin=raw_html.indexOf(">", t);
    var end=raw_html.indexOf("<", begin);
    name=raw_html.substring(begin+1, end);

    t=raw_html.indexOf("nowrap>Period");       //finds period
    //console.log(t);
    begin=raw_html.indexOf(":", t);
    end=raw_html.indexOf("<img", begin);
    period=raw_html.substring(begin+2, end);

    id=name+period;
}

function update(){                                  //This is called when the user modifies the value of the final, and closes dialog box
    console.log("update");
    var display_box=document.getElementById("type_final");
    if(display_box.options[display_box.selectedIndex].value=="percent"){
        value_final_percent=document.getElementById("user_final").inputbox.value;
        localStorage["value_final_percent"+id]=value_final_percent;
        console.log(value_final_percent);
        if(start){
            current_grade=cpoints/tpoints;
        }
        update_percent();
    }
    if(t.options[t.selectedIndex].value=="points"){
        if(start==-1){
            final_points=document.getElementById("user_final").inputbox.value;
            localStorage["fpoints"+id]=final_points;
            //console.log(tpoints);
            update_points();
        }   
    }
}

function update_percent(){                          //This modifies the raw_html to display the percentage needed, ONLY percent based finals.
    var cp=((current_grade/100.0)*(1.0-(value_final_percent/100.0))).toFixed(4);
    needed_grade.innerHTML="";
    t=((.9-cp)/(value_final_percent/(100.0*100.0))).toFixed(2);
    needed_grade.innerHTML+="A: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp"
    t=((.8-cp)/(value_final_percent/(100.0*100.0))).toFixed(2);
    needed_grade.innerHTML+="B: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
    t=((.7-cp)/(value_final_percent/(100.0*100.0))).toFixed(2);
    needed_grade.innerHTML+="C: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
    t=((.6-cp)/(value_final_percent/(100.0*100.0))).toFixed(2);
    needed_grade.innerHTML+="D: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
}

function update_points(){
    var cp=cpoints;
    var fp=parseInt(final_points);
    needed_grade.innerHTML="";
    t=((.9*(tpoints+fp)-cp)/final_points*100.0).toFixed(2);
    needed_grade.innerHTML+="A: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
    t=((.8*(tpoints+fp)-cp)/final_points*100.0).toFixed(2);
    needed_grade.innerHTML+="B: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
    t=((.7*(tpoints+fp)-cp)/final_points*100.0).toFixed(2);
    needed_grade.innerHTML+="C: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
    t=((.6*(tpoints+fp)-cp)/final_points*100.0).toFixed(2);
    needed_grade.innerHTML+="D: "+((t<0)?"Impossible ":t+"&nbsp")+"&nbsp&nbsp";
}

function adjustFinal(){                         //creates a dialog box whenever the button afbutton is pressed.
    /*var link = document.getElementById("afbutton");
    link.addEventListener("click", function(){
        alert("HI");
    })*/
    //prompt("Enter a value:", "");
    $("#popup").dialog({
                draggable: false, 
                modal: true, 
                position: {my: "center", at: "center"}, 
                buttons: {"Done": function(){$(this).dialog("close"); update()}}
                });
}

if (document.title.indexOf("Progress Report") != -1) {
    find_values();
    if(("value_final_percent"+id) in localStorage){          //checks to see if a final percentage has already been manually entered.
        value_final_percent=parseInt(localStorage["value_final_percent"+id]);
    }
    if(("fpoints"+id) in localStorage){
        final_points=localStorage["fpoints"+id];
        update_points();
    }
    console.log(start);
    if(!is_weighted){
        update_percent();
    }
    $("<button/>", {id: "afbutton", click: adjustFinal, text: "Click to adjust finals value"}).appendTo("#page_title");

}
share|improve this question

1 Answer 1

up vote 1 down vote accepted

Ok a few things.

JSHint

First I'd run the code through jshint to conform to get some of the easy stuff out of the way. Depending on what editor you're using there are often plugins that will validate your JS with JSHint.

Variable Names

The general convention in javascript is to use camelcase for variable naming. All built in javascript functions are done in camelcase (i.e. document.getElementById).

Variable Declaration

I'd also be more consistent with where you declare your variables. They should all be at the top of the function due to hoisting. It's also good practice to declare a javascript namespace so that you do not create a bunch of global variables like you have. Always use var before declaring a variable.

jQuery

It looks like you're using jQuery in your document, so you might as well take advantage of some of the utilities that it comes with to make your life easier.

Cache the $('#popup') in a variable var $popup = $('#popup') so you don't have to keep reselecting the element. Use jQuery's html function instead of innerHTML (ie $popup.html('myhtmlstring').

Use jQuery selectors if you've included it in the page. No need to use getElementById.

Use jQuery's $.get function instead of your own http get method.

Other

<div/> is not valid html. It should be <div></div>.

if(start!=-1){
    is_weighted=true;
} else{
    is_weighted=false;
}   

Can be simplified to is_weighted = start !== -1

Please remove console.logs and comments before code review.

exist function is unused and not useful.

There seems to be a lot of code here and it's a bit out of scope to refactor logic, but I'd say that this could be more concise and better written by trying to separate logic from presentation. Right now your html is mixed right in with all this crazy logic which makes the code very unmaintainable.

Hope this gives you something to start with.

share|improve this answer
    
+1, great answer as your first answer on CodeReview! –  rolfl Jan 12 '14 at 14:02
    
Thanks. What do you mean by separating logic from presentation? Like should I store the html code in variables, and use those in functions? –  horace he Jan 14 '14 at 6:02
    
It's the idea that functions that have to do with logic should just contain logic and then use the output of those to determine what the presentation is. The philosophy is that the presentation changes much more rapidly than the logic so it's good to separate them to reduce bugs. I'd do a google search on 'MVC' or 'separation of presentation from logic' to find a good guide. –  benr Jan 15 '14 at 8:23

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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