patternjavascriptMinor
Serial port for Node.js
Viewed 0 times
nodeserialforport
Problem
I am trying to create a class for serial port using Node.js, but I want to know if there is a better way to write my data class code.
In my sample code below, in the
Is this a good way of declaring this port object? I think it is not good because it is difficult for users to see what the objects' members contain. Is there any way to make it clearly expose the port object member? Or is there a better way to improve this code structure?
In my sample code below, in the
getPortName prototype in the forEach loop, I declare a port object to store the serial port information before pushing into an array.Is this a good way of declaring this port object? I think it is not good because it is difficult for users to see what the objects' members contain. Is there any way to make it clearly expose the port object member? Or is there a better way to improve this code structure?
'use strict';
var serialPort = require("serialport");
function Serial() {
this.ComName = "";
}
Serial.prototype.getPortName = function() {
serialPort.list(function (err, ports) {
var availablePorts = [];
ports.forEach(function(port) {
var port = {
comName: "",
manufacturer: "",
pnpId:
}
port.comName = port.comName;
port.manufacturer = port.manufacturer;
port.pnpId = port.pnpId;
availablePorts.push(port);
});
});
return availablePorts;
};
Serial.prototype.setCOMName = function(name) {
this.ComName = name;
}
module.exports = Serial;Solution
First off, you should probably allow a default
Next,
In classical OOP parlance, this is a
With that change, you can simply call
Furthermore, your naming conventions are a little odd. You use
Finally, these changes beg the question... why is this object-oriented code at all? The only state you appear to be representing is a single string with no actual methods on it. Why not just export
ComName to be passed into the constructor, and the default should probably be null instead of an empty string (unless you have good reason to do otherwise).function Serial(name) {
this.ComName = name !== undefined ? name : null;
}Next,
getPortName is strange. You set it on Serial.prototype, which indicates it will be used as an instance method. In fact, however, it doesn't use instance state at all! As your code is currently laid-out, you would need to perform the following to call getPortName:new Serial().getPortName()In classical OOP parlance, this is a
static function. It doesn't need instance state, so declare it on Serial itself, not Serial.prototype.Serial.getPortName = function() { ... }With that change, you can simply call
Serial.getPortName(), as you would a static method in other languages.Furthermore, your naming conventions are a little odd. You use
ComName for the property but COMName for the setter. Use comName for the property for consistency. If you need the setter, use setComName. However, the setter is also unnecessary. Why not just access serial.comName directly?Finally, these changes beg the question... why is this object-oriented code at all? The only state you appear to be representing is a single string with no actual methods on it. Why not just export
getPortName and call it a day? If your code is more complex, using JavaScript's OO capabilities might be worth it, but otherwise, you don't need it. A simple function will do.Code Snippets
function Serial(name) {
this.ComName = name !== undefined ? name : null;
}new Serial().getPortName()Serial.getPortName = function() { ... }Context
StackExchange Code Review Q#78845, answer score: 5
Revisions (0)
No revisions yet.