HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Flexible presentation of a user's name based on a combination of available properties

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
combinationavailablepresentationpropertiesuserflexiblenamebased

Problem

The goal is to add a new displayValue property to each object depending on what it has available out of Firstname, Lastname, Nickname and Name.

I'd like this cleaned up.

```
var data = [
{
Id: 132,
Name: null,
Firstname: "Sam",
Lastname: null,
Nickname: "Dim",

},
{
Id: 162,
Name: null,
Firstname: "Junior",
Lastname: "Dimmer",
Nickname: null,
},
{
Id: 132,
Firstname: null,
Lastname: null,
Nickname: "Dimmest",
},
{
Name: "No Light",
Id: 132,
Firstname: null,
Lastname: null,
Nickname: null
}
]

function formatPeople(data) {

var yesProp = function (prop) {
return data[i].hasOwnProperty(prop) && data[i][prop] !== null;
}

var noProp = function (prop) {
return data[i].hasOwnProperty(prop) && data[i][prop] === null;
}

for (var i = 0, len = data.length; i < len; i += 1) {
// if it has the property Name
if (yesProp('Name')) {
data[i].displayValue = data[i].Name;
}
if (yesProp('Lastname')) {
if (yesProp('Firstname')) {
// if it has the property Lastname and Firstname
data[i].displayValue = data[i].Lastname + ', ' + data[i].Firstname;
} else {
if (yesProp('Nickname')) {
// if it has the property Lastname and Nickname but not Firstname
data[i].displayValue = data[i].Lastname + ', ' + data[i].Nickname;
} else {
// if it has the property Lastname but not Firstname or Nickname
data[i].displayValue = data[i].Lastname;
}
}
} else {
if (yesProp('Firstname')) {
if (yesProp('Nickname')) {
// if it does not have the property Lastname but does have Firstname and Nick

Solution

First

Idiomatic javascript is to camelCase property names. consider the following:

{
    name: "No Light",
    id: 132,
    firstname: null,
    lastname: null,
    nickname: null
}


Your yesProp and noProp variables are very bad design. Hoisting allows the i variable to be in scope through es3 (to my recollection), but you should never try to take advantage of that. These functions should be rewritten to take the object they are operating on as a parameter.

Additionally, unless there should be some third undefined state, the opposite of yesProp should be !yesProp, not a different detection method.

yesProp is a poor description for the method as well. Consider something describing what it does like hasProperty.

Consider the following:

function hasNotNullProperty(target, property) {
    return target.hasOwnProperty(property) && target[property] !== null;
}


Second

Javascript is a truthy language, undefined and null are considered false. Your yesProp and my hasNotNullProperty method can go away entirely with a simple if (target[property]) { / do stuff / }. If the property is undefined or evaluates to a falsy value, then it will not execute the block.

Again in this section, data is a poor description of what we are expecting. Consider using a more descriptive value such as people.

There is no need to return data; at the end of the function -- You've modified the objects that were passed in.

Consider the following:

function formatPeople(people) {
    for (var i = 0, len = people.length; i < len; i += 1) {
        var displayName;

        // Lastname was defined and not null
        if (people[i].lastname) {
            displayName = people[i].lastname;
            // Firstname was defined and not null
            if (people[i].firstname) {
                displayName+= ', ' + people[i].firstname;
            }
            // Firstname was not defined or was null, but Nickname is defined and not null
            else if (data[i].nickname) {
                displayName += ', ' + people[i].nickname;
            }
        }
        // Lastname was undefined or null, but Firstname is defined and not null
        else if (people[i].firstname) {
            displayName = people[i].firstname;
            if (people[i].Nickname) {
                displayName += ', ' + people[i].nickname;
            }
        }
        // Lastname and Firstname were undefined or null, Nickname is defined and not null
        else if (people[i].nickname) {
            displayName = people[i].nickname;
        }

        if (displayName) {
            people[i].displayValue = displayName
        }
    }
}


That still leaves a lot of duplicate code. We will want to clean that up.
Finally

The truthy nature of javascript allows us to do this as well:

var falsy = null;
var truthy = {}; // an anonymous object
var isDefinedAndNotNull = undefined || falsy || truthy; // assigns truthy to isDefinedAndNotNull


This allows us to short-circuit the assignment of a value such as a name.

Let's remove some of that duplicate code, and make a few more names clearer.

function setDisplayNamesForPeople(people) {
    var lastname, firstname, nickname, defaultName, displayName;

    for (var i = 0, length = people.length; i < length; i++) {
        lastname = people[i].lastname;
        firstname = people[i].firstname;
        nickname = people[i].nickname;
        defaultname = people[i].name;
        displayName = formatName(lastname, firstname, nickname, defaultname);

        if (displayName) {
            people[i].displayValue = displayName;
        }
    }
}

//formatName format is (lastname [', ' (firstname | nickname)] | (firstname [', ' nickname]) | nickname | defaultname)
function formatName(lastname, firstname, nickname, defaultname) {
    var formattedName = "";
    if (lastname && (firstname || nickname)) {
        formattedName = ', ' + (firstname || nickname);
    }
    else if (firstname && nickname) {
        formattedName = ', ' + nickname;
    }

    formattedName = (lastname || firstname || nickname || defaultname) + formattedName;

    return formattedName;
}

Code Snippets

{
    name: "No Light",
    id: 132,
    firstname: null,
    lastname: null,
    nickname: null
}
function hasNotNullProperty(target, property) {
    return target.hasOwnProperty(property) && target[property] !== null;
}
function formatPeople(people) {
    for (var i = 0, len = people.length; i < len; i += 1) {
        var displayName;

        // Lastname was defined and not null
        if (people[i].lastname) {
            displayName = people[i].lastname;
            // Firstname was defined and not null
            if (people[i].firstname) {
                displayName+= ', ' + people[i].firstname;
            }
            // Firstname was not defined or was null, but Nickname is defined and not null
            else if (data[i].nickname) {
                displayName += ', ' + people[i].nickname;
            }
        }
        // Lastname was undefined or null, but Firstname is defined and not null
        else if (people[i].firstname) {
            displayName = people[i].firstname;
            if (people[i].Nickname) {
                displayName += ', ' + people[i].nickname;
            }
        }
        // Lastname and Firstname were undefined or null, Nickname is defined and not null
        else if (people[i].nickname) {
            displayName = people[i].nickname;
        }

        if (displayName) {
            people[i].displayValue = displayName
        }
    }
}
var falsy = null;
var truthy = {}; // an anonymous object
var isDefinedAndNotNull = undefined || falsy || truthy; // assigns truthy to isDefinedAndNotNull
function setDisplayNamesForPeople(people) {
    var lastname, firstname, nickname, defaultName, displayName;

    for (var i = 0, length = people.length; i < length; i++) {
        lastname = people[i].lastname;
        firstname = people[i].firstname;
        nickname = people[i].nickname;
        defaultname = people[i].name;
        displayName = formatName(lastname, firstname, nickname, defaultname);

        if (displayName) {
            people[i].displayValue = displayName;
        }
    }
}

//formatName format is (lastname [', ' (firstname | nickname)] | (firstname [', ' nickname]) | nickname | defaultname)
function formatName(lastname, firstname, nickname, defaultname) {
    var formattedName = "";
    if (lastname && (firstname || nickname)) {
        formattedName = ', ' + (firstname || nickname);
    }
    else if (firstname && nickname) {
        formattedName = ', ' + nickname;
    }

    formattedName = (lastname || firstname || nickname || defaultname) + formattedName;

    return formattedName;
}

Context

StackExchange Code Review Q#69648, answer score: 2

Revisions (0)

No revisions yet.