Problem
So I have created this function that will add a disk if needed and it takes a disk and a size as inputs. The idea is that I need to update data.disks with a few properties. Disk is called from an external request and it follows the pattern of “disk1”, “disk2” and so on. Looking at it there are a few hard-coded values and some that are null. However I left them on purpose to show the structure of data.disk for the next person that might read it or modify it. Because of this it looks verbose and I don’t really like it all that much.
Do you see a way to make this function more readable/better in any way?
function addDisk(disk, size) {
var diskNumber = disk.slice(-1);
var diskLabel = "Hard disk " + diskNumber;
var diskIndex = diskNumber - 1;
//Updating JSON data object
data.disks[diskIndex] = {
"componentTypeId" : "com.vmware.csp.iaas.blueprint.service",
"componentId" : null,
"classId" : "Infrastructure.Compute.Machine.MachineDisk",
"typeFilter" : null,
"data" : {
"capacity" : size,
"custom_properties" : null,
"id" : null,
"initial_location" : "",
"is_clone" : false,
"label" : diskLabel,
"storage_reservation_policy" : "",
"userCreated" : true,
"volumeId" : diskIndex
}
}
log(diskLabel + " has capacity of: " + size + " GBs");
}
Solution
You could define a default disk object, so you can show all fields on default values and document the values allowed for each:
var DiskSkelethon = {
"componentTypeId" : "com.vmware.csp.iaas.blueprint.service",
"componentId" : null,
"classId" : "Infrastructure.Compute.Machine.MachineDisk",
"typeFilter" : null,
"data" : {
"capacity" : -1,
"custom_properties" : null,
"id" : null,
"initial_location" : "",
"is_clone" : false,
"label" : "",
"storage_reservation_policy" : "",
"userCreated" : true,
"volumeId" : -1
}
}
Then in your function you will make a copy of it and override just the fields you’re changing:
function addDisk(disk, size) {
var diskNumber = parseInt(disk.slice(-1), 10);
if (isNaN(diskNumber)) {
log("some error...");
return false;
}
var diskLabel = "Hard disk " + diskNumber;
var diskIndex = diskNumber - 1;
//Updating JSON data object
var diskData = Object.assign({}, DiskSkelethon)
diskData.data.capacity = size;
diskData.data.label = diskLabel;
diskData.data.volumeId = diskIndex
data.disks[diskIndex] = diskData
log(diskLabel + " has capacity of: " + size + " GBs");
}
I think this code is fine, if you use it professionally I would
- Document that size must be provided in Gigs
- Verify that size actually is a number, or can be parsed as a number, throw an exception otherwise
- Document how the parameter
disk
must be formatted -
Verify that
disk.slice(-1)
is a number, or can be parsed as a number, throw an exception otherwise -
Note that
data.disks
seems a global variable, personally, I would have prefered to see a setter function for updating a disk object.