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 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;
1 Answer 1
First off, you should probably allow a default 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.
-
\$\begingroup\$ thank you for your clear explanation, i have question about your setter and getter, you mention that i could just access it directly instead of using setter or getter, but i do read that it is beneficial for me to use setter and getter for my class member? 2nd) how do i represent my data class (the port object contains name, pnpid..) how could i make this data class clear and be reused? \$\endgroup\$Tim– Tim2015年01月28日 06:27:38 +00:00Commented Jan 28, 2015 at 6:27
-
\$\begingroup\$ @Tim That's a number of questions, and I don't plan to answer them in the comments of this answer. I will say that with the near-ubiquitous support for
Object.defineProperty
, using custom properties is probably a better replacement for getters and setters most of the time. \$\endgroup\$Alexis King– Alexis King2015年01月28日 06:31:18 +00:00Commented Jan 28, 2015 at 6:31
Explore related questions
See similar questions with these tags.