[Code Review] generating a username from user input

JavaScript
1

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

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

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:


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;

3

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

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.


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.


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.


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;
}