My boss just mopped the floor with me for being overly verbose and complicated in creating asynchronous code. The point of the code is to create a chain of sequential promises where one promise strictly comes after the other – each of them is used to invoke the read of a part of a large array from a remote device (you have to give the device a list of id’s to get their contents back).
{ //inside a TypeScript class method....
//.....
var Data = [];
for (var i in requestData) {
var obj = requestData[i];
Data.push({ /* application specific rewrite of object */ });
}
const RPM_AsyncWork = (prop) => {
// do some work with the received [chunk of the whole
// array] in prop
};
const RPM_Promise = (RpmChunk) =>
new Promise((resolve, reject) => {
this.MyDevice.ReadMultiple(RpmChunk, (err, prop) => {
if (err) {
reject("Error ReadMultiple: " + err);
} else {
RPM_AsyncWork(prop);
resolve(0);
}
});
});
(async () => {
// Data contains the complete list of all id's to be read
try {
for (let i = 0; i < Data.length; i += this.RpmChunkSize.Value) {
let RpmChunk = Data.slice(i, i + this.RpmChunkSize.Value);
await RPM_Promise(RpmChunk);
}
} catch (err) {
this.Error("Error: " + err);
}
})();
//...
// ... other method code
}
He then rewrote my code to this form:
{ //inside a TypeScript class method....
//.....
var DataChunk = [];
var Data = [];
DataChunk.push(Data);
let count = 0;
for (var i in requestData) {
let obj = requestData[i];
if (count > this.RpmChunkSize.Value) {
Data = [];
DataChunk.push(Data);
count = 0;
}
Data.push({ /* application specific rewrite of object */ });
count++;
}
var start = Date.now();
DataChunk.forEach(async (data) => {
await this.ReadPropertyMultipleAsync(data);
});
//...
// ... other method code
}
async ReadMultipleAsync(data): Promise<any> {
this.MyDevice.ReadMultiple(data, (err, prop) => {
if (err) {
this.Error("Error ReadMultiple: " + err);
} else {
try {
// do some real stuff with 'prop'
return true;
} catch (err) {
this.Error("Error decode ReadMultiple: " + err);
}
}
return false;
});
}
The MyDevice.ReadMultiple
function does return only a number
between 0 and 255 as integer error code. It is guaranteed not multi-interface because I wrote it myself. It is a node-addon function in C++ and executes the callback asynchronously when the network response or an error has arrived (although I think this is immaterial for my question).
What was bad in his judgement:
- that I created some
const
functions because they “are not object oriented” and consume time when the executor has to parse/run through them each method invocation - that my program flow is confusing
- that I isolated the “work after successful reception” in an extra piece of code
My questions: are his criticisms valid (or valid enough, however you want to put it)?
Also: I never saw a promise being created this way, implicitly, by employing the callback just as a true/false
generator. The TypeScript compiler accepts this silently. What is going on here? How can I fancy the resolve
and reject
paths that I know from all other examples?
8