SitePoint Sponsor

User Tag List

Results 1 to 4 of 4

Hybrid View

  1. #1
    SitePoint Member
    Join Date
    Nov 2011
    Location
    The Colony, TX
    Posts
    17
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    [Code Review] generating a username from user input

    I run a VERY small business, where I'm building an intranet for our employees to use (add clients, generate invoices, things like that) I needed to generate an employee's username based on their first and last name. My original goal was to include verification, to make sure we aren't generating a username that already exists, but that can come later, I've only got 29 employees right now, and my methods won't create conflicts, yet.......
    its a very simple javascript function I call genid to take a user's input based on first and last name, and spit out a username and an email address, which are plugged into readonly fields in the same form

    Code:
    function genid() {
    		var a = document.getElementById('fname').value
    		var b = document.getElementById('lname').value
    		var c = a.substring(0,1);
    		var d = (c + b);
    		var e = d.toLowerCase();
    		var f = '@email.com';
    		var g = (e + f);
    		document.getElementById('uname').value = (e);
    		document.getElementById('email').value = (g);
    		}
    And then, in my form, once a user fills out the lname field, I call genid via an onblur, and the appropriate username and email fields are filled in. The code works as designed, but after declaring 7 variables, I realize its probably sloppy, and could have been more efficiently carried out. I'm not a web designer, but I like to do things "right" nonetheless, so any pointers on cleanup would be appreciated.

  2. #2
    Utopia, Inc. silver trophy
    ScallioXTX's Avatar
    Join Date
    Aug 2008
    Location
    The Netherlands
    Posts
    9,097
    Mentioned
    153 Post(s)
    Tagged
    2 Thread(s)
    The problem I see with the code is that it's pretty much unreadable because the variable names don't make any sense, i.e., don't describe what they're for. Also you have quite a bit of temporary values you could do away with. I would personally change it to:

    Code:
    function genid() {
      var
        fname = document.getElementById('fname').value,
        lname = document.getElementById('lname').value,
        alias = (fname.substring(0,1) + lname).toLowerCase()
      ;
      document.getElementById('uname').value = alias;
      document.getElementById('email').value = alias + '@email.com';
    }
    This way it's clear (IMHO) what's what, and I don't have to go "(e + f), hold on, what was e again? and f?", etc

    PS. The var a=1, b=2, c=3; is just a shorthand for var 1=a; var b=2; var c=3;
    Rémon - Hosting Advisor

    SitePoint forums will switch to Discourse soon! Make sure you're ready for it!

    Minimal Bookmarks Tree
    My Google Chrome extension: browsing bookmarks made easy

  3. #3
    SitePoint Member
    Join Date
    Nov 2011
    Location
    The Colony, TX
    Posts
    17
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks for the input, you're right, I like the trimmed down version better. And, by using descriptive variable names, when I have to modify this (and I know I will) it will make more sense two or three months down the road.

  4. #4
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,729
    Mentioned
    104 Post(s)
    Tagged
    4 Thread(s)
    Quote Originally Posted by m.andrew.dennis View Post
    Thanks for the input, you're right, I like the trimmed down version better. And, by using descriptive variable names, when I have to modify this (and I know I will) it will make more sense two or three months down the road.
    Let's simplify the task of the function, by splitting it up in to a couple of functions, each with well defined tasks.

    The generateUserDetails() function takes a first and last name, and returns an object that contains the username and email address.

    Code javascript:
    function generateUserDetails(firstname, lastname) {
        var uname = (firstname.substring(0,1) + lastname).toLowerCase();
        return {
            uname: uname,
            email: uname + '@email.com'
        };
    }
    function genid() {
        var userDetails = generateUserDetails(
            document.getElementById('fname').value,
            document.getElementById('lname').value
        );
        document.getElementById('uname').value = userDetails.uname;
        document.getElementById('email').value = userDetails.email;
    }

    Whether this next piece works for you depends on how your form triggers the genid() function.

    The genid() function can be cleaned up too, depending on how it's called.
    If for example a traditional event assignment is used to assign genid to an event, the this keyword within the genid() function will refer to the form element that triggered the event, so that you can use form elements within the function, instead of element identifiers.

    Code javascript:
    var form = document.getElementById('createUser');
    form.elements.lname.onblur = genid;

    Fundamentally, you want to avoid using unique identifiers within your script as much as possible. In this situation, only the form itself as a whole should be identified by a unique identifier, so your form is in less danger of breaking when layout changes are made to your form.

    Code javascript:
    function generateUserDetails(firstname, lastname) {
        ...
    }
    function genid() {
        var form = this.form;
        var userDetails = generateUserDetails(
            form.elements.fname.value,
            form.elements.lname.value
        );
        form.elements.uname.value = userDetails.uname;
        form.elements.email.value = userDetails.email;
    }
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript


Tags for this Thread

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •