Refactor the code
Closed this issue · 7 comments
Currently, the code is in shambles, so
As a programmer
I want a clearer code
So that so it can help me read it and add more features more easily
If you want suggestions for your code i would say fix the await and .then usage.
You are creating variables that you dont use in your functions such as const GPU = await sysinf.graphics().then((data) => {}
Since you are using .then, you dont need an async function and the variable you create.
Either use async/await or .then, not both.
I personally like async/await better.
I would write it somewhat like this:
async function getSystemInformation() {
const gpu = await sysinf.graphics();
let displays = '';
let graphicsCards = '';
gpu.controllers.forEach((controller) => {
graphicsCards += `${controller.model} `;
})
gpu.displays.forEach((display) => {
displays += `${display.resolutionx}x${display.resolutiony} `
})
return {
gpuInfo: graphicsCards,
displays: displays
}
}
You're right, I was supposed to choose async/await because it's newer but when started implementation I wasn't able to make it work, so I kind of mixed them together to make the it work and forgot to fix it the next day.
I didn't remove .then() when using Inquirer, because it's the way they're using it in Inquirer readme
Sorry, didn't get any warning for this. (forgot to watch it)
But the async code looks much cleaner now :)
if you want more suggestions:
This kinda depends on how you want to do this, but i see that you created a global object systemInfo
.
This object already has the keys and values assigned as the new variables you are creating in the getSystemInformation()
function.
You should be able to do something like this:
async function getSystemInformation() {
const GPU = await sysinf.graphics();
GPU.controllers.forEach((elem) => {
systemInfo.graphicsInfo.gpuInfo += `${elem.model} `;
})
GPU.displays.forEach((elem) => {
systemInfo.graphicsInfo.displays += `${elem.resolutionx}x${elem.resolutiony} `
})
const memory = await sysinf.mem();
systemInfo.memoryInfo.total = memory
? (memory.total * 9.537 * Math.pow(10, -7)).toFixed(0)
: null; //conversion to Mb
systemInfo.memoryInfo.used = memory
? (memory.used * 9.537 * Math.pow(10, -7)).toFixed(0)
: null;
systemInfo.memoryInfo.free = memory
? (memory.free * 9.537 * Math.pow(10, -7)).toFixed(0)
: null;
const osInfo = await sysinf.users()
systemInfo.osInfo.username = osInfo
? osInfo[0].user
: null;
systemInfo.osInfo.shell = osInfo
? osInfo[0].tty
: null;
return systemInfo;
}
I would also refrain from writing a variable name in all caps. I recommend camelCase or PascalCase.
Also to add to your comment earlier:
I checked inquirer and the .prompt
method returns a promise. That means you can use async/await here as well.
Thanks for feedback!
Regarding the all-caps variable. I use camelCase throughout the whole project, I just got confused when naming abbreviations of words(ex. GPU - Graphics Processing Unit), which are usually written in all-caps.