patternjavascriptMinor
Flexible presentation of a user's name based on a combination of available properties
Viewed 0 times
combinationavailablepresentationpropertiesuserflexiblenamebased
Problem
The goal is to add a new
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
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:
Your
Additionally, unless there should be some third undefined state, the opposite of
Consider the following:
Second
Javascript is a truthy language,
Again in this section,
There is no need to
Consider the following:
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:
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.
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 isDefinedAndNotNullThis 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 isDefinedAndNotNullfunction 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.