6
\$\begingroup\$

I'm looking for a more cleaner and simpler solution to auto-format a text-box field intended only for the user to enter their college Grade Point Average (GPA). Here is my code:

<label for="collegeGPA">GPA:</label>
<input type="text" name="collegeGPA" id="collegeGPA" maxlength="4" style="width:45px;">

<script type="text/javascript"> 

//<!-- When executed this script [BELOW] auto-formats a Grade Point Average (GPA) field given the element ID [BEGIN] -->

function FormatGPA(gPAElementID) {

    re = /\D/g; // remove any characters that are not numbers
    gPAVal = document.getElementById(gPAElementID).value.replace(re,"");
    gPAValFirstCharacter = gPAVal.charAt(0);

    if ( ( gPAVal >= 0 ) &&  ( gPAValFirstCharacter < 5 ) )
        {

            gPALen = gPAVal.length;

            if( gPALen > 1)
                {
                    gPAa=gPAVal.slice(0,1);
                    gPAb=gPAVal.slice(1);
                    document.getElementById(gPAElementID).value = gPAa + "." + gPAb; 
                }
        else
                { 
                    document.getElementById(gPAElementID).value=gPAVal;
                };

        }

    else
        { 
            document.getElementById(gPAElementID).value="";
        };

};

//<!-- When executed this script [ABOVE] auto-formats a Grade Point Average (GPA) field given the element ID [END] -->


document.getElementById('collegeGPA').onblur = function (e) { FormatGPA(this.id); };
document.getElementById('collegeGPA').onfocus = function (e) { FormatGPA(this.id); };
document.getElementById('collegeGPA').onkeypress = function (e) { FormatGPA(this.id); };
document.getElementById('collegeGPA').oninput = function (e) { FormatGPA(this.id);  };
document.getElementById('collegeGPA').onchange = function (e) { FormatGPA(this.id); };

</script>

...and here is my JSFiddle: http://jsfiddle.net/JamesAndersonJr/kxaBJ/1/ I'm looking for a simpler way to format the GPA input dynamically (i.e. as the user types it in). The format should be:

  • 1st Character: any digit 0-4

  • 2nd Character: Always a Period (should also be auto-inserted after user types a valid first digit)

  • 3rd Character: any digit

  • 4th Character: any digit

UPDATE: Lowest possible GPA is 0.00, not 1.00.

\$\endgroup\$
3
\$\begingroup\$

First of all, I'd be hesitant to use as-you-type correction to begin with. It doesn't take much for something like that to be an annoyance rather than a help. It's probably better to validate the input only when it loses focus, or when the form is about to be submitted, and then inform the user somehow.

Point is, it's not hard make your code trip over something. For instance, I just pasted in "2312" and got a GPA with 3 decimal places. I also tried typing in a valid value, and then went back with the arrow keys and removed the first digit with the intention of replacing it - but removing that first digit cleared the entire input. If I just select the first digit, and hit a key, my input gets ignored, valid or not. I can't manually enter a decimal point. The cursor jumps around... and so on.

Unless your system is perfect and takes all possible interactions into account, it'll likely be very annoying and frustrating to use.

Point is, it's a lot easier to validate stuff, once it's all been entered. For instance, this regular expression /^([0-3](\.\d\d?)?|4(.00?)?)$/ will match a valid GPA with 0-2 decimal places.

But don't change the value! You can format it, if it's otherwise valid (like adding ".00" if the user just typed "4"), but leave it at that. Just call attention to the error, optimally with an explanation of why the value is invalid.

In any event, presuming this is being sent to a server, always, always validate it on the server. Never trust the client™

But there's still code to review here, regardless of its use.

Code notes:

  • Declare your variables with var. Right now, every variable is global! Big no-no.
  • Don't find and re-find an element with getElementById; find it once, stick it in a variable, and use that instead.
  • Don't use the old-school .onblur = etc. event handling. Favor addEventListener instead (or use a library to do the event handler registration for maximum compatibility)

More broadly, there's no need to keep declaring a function like function (e) { FormatGPA(this.id); } for each event. Declare that function once, and use it for every event. Or simply don't use an argument for the format function, and let it know what element to find. Or, if you use addEventListener, the element will be available as this in the event handler, i.e.:

var someInput = document.getElementById("some-input");
someInput.addEventListener("click", someHandler, false);

function someHandler() {
  alert(this.value); // `this` refers to the element that triggered the event
}

(using alert just as an example - it's obviously not an ideal way to provide feedback.)

Style notes:

  • Function names and variables should be lowerCamelCase (unless they are constructors), so: formatGPA. And if you've got an acronym at the start of your name, make it all-lowercase, i.e. gpaElementId.
  • Don't use HTML comment syntax in your JS code; it doesn't matter, and makes little sense.
  • Avoid the brace-on-new-line style. JavaScript has automatic semicolon insertion, which (long story short) means that you should stick to a style like

    if(condition) {
    
    } else {
    
    }
    
\$\endgroup\$
  • \$\begingroup\$ I've improved upon and updated my code, taking into account some of the things you've said. Now, about auto-formatting while typing: I'm creating a "Smart-Form" that never lets the user input bad values in the first place instead of nagging them with 100's of ["silencable" in most browsers] alert boxes while they're trying to submit the form. \$\endgroup\$ – James Anderson Jr. Apr 2 '14 at 22:10
  • 1
    \$\begingroup\$ @JamesAndersonJr. I'm glad you found it helpful - however, please don't update your question (I'll roll it back to its previous state). When you do, answers lose their context. If you've got updated code you'd like reviewed, please post a new question instead. It also gives others a chance to offer their advice. As for nagging users: I never imagined using alert boxes. Simply highlight the invalid input(s) (e.g. with a CSS class), and let the user correct any errors before proceeding. It doesn't have to be obtrusive. \$\endgroup\$ – Flambino Apr 3 '14 at 0:10
  • \$\begingroup\$ Why I have to repost my question, and in the process be guilty of spamming CR.SE - I don't know. Poor system of operation. I already have a question open. If someone search Google for this question, they may be mislead by an old solution, and to add insult to injury, the first question can't be deleted. Someone from moderators or admin, please contact me (personally) and explain more clearly how this site works, because I'm confused and it's just not making much sense to me. \$\endgroup\$ – James Anderson Jr. Apr 3 '14 at 5:24
  • \$\begingroup\$ @JamesAndersonJr. You are, in a sense, supposed to spam the site. It's a Q&A site: If someone finds your question, yes, they might see outdated code, but they'll see equally outdated answers. The point of a Q&A format is that the Qs and the As fit together. \$\endgroup\$ – Flambino Apr 3 '14 at 6:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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