Overuse of Promises

Dear all,

I am continuing my deep dive into the Volumio code and discovering some truly curious things :slight_smile:

IMHO there is some extremely weird use of promises all over the place.

A promise should be used for asynchronous calls and not as a way of chaining synchronous code.

Here is a small example.

[code]CoreStateMachine.prototype.getState = function() {
var self = this;
self.commandRouter.pushConsoleMessage(’[’ + Date.now() + '] ’ + ‘CoreStateMachine::getState’);
var sService = null;
if (‘service’ in self.currentTrackBlock) {
sService = self.currentTrackBlock.service;
}

return libQ.resolve({
	status: self.currentStatus,
	position: self.currentPosition,
	title: self.currentTitle,
	artist: self.currentArtist,
	album: self.currentAlbum,
	albumart: self.currentAlbumArt,
    uri: self.currentUri,
	seek: self.currentSeek,
	duration: self.currentDuration,
	samplerate: self.currentSampleRate,
	bitdepth: self.currentBitDepth,
	channels: self.currentChannels,
	random: self.currentRandom,
	repeat: self.currentRepeat,
	volume: self.currentVolume,
	mute: self.currentMute,
	stream: self.isStreaming,
	service: sService
});

};[/code]

Another example

[code]CorePlayQueue.prototype.getTrackBlock = function (nStartIndex) {
this.commandRouter.pushConsoleMessage(’[’ + Date.now() + '] ’ + ‘CorePlayQueue::getTrackBlock’);

var sTargetService = this.arrayQueue[nStartIndex].service;
var nEndIndex = nStartIndex;
var nToCheck = this.arrayQueue.length - 1;

while (nEndIndex < nToCheck) {
	if (this.arrayQueue[nEndIndex + 1].service !== sTargetService) {
		break;
	}
	nEndIndex++;
}

var arrayUris = libFast.map(this.arrayQueue.slice(nStartIndex, nEndIndex + 1), function (curTrack) {
	return curTrack.uri;
});

return libQ.resolve({service: sTargetService, uris: arrayUris, startindex: nStartIndex});

};[/code]

In both cases the code is synchronous and yet the return value is returned as the result of a resolved promise. This is downright confusing and might concievably cause wierd issues if .then methods are called a tick later or something (I don’t know if that happens).

Then later on, instead of simple code like this (which would work in exactly the same way)

self.funca(); self.funcb();

we get something like this

libQ.defer(self.funca.bind(self)).then(self.funcb.bind(self));

There are examples of code like this in there!

Anyway it just confuses the issue is less efficient and hides where asyncronous code is actually needed.

I am going through and trying to remove examples like this. I will submit some pull requests.

What are peoples opinions on this? Is there a reason for this use of promises?

This part was built at the very beginning of Volumio2, when node didn’t have a solid base for sync operations, and ning-yu IMHO did an excellent job with the kew library to achieve such complex tasks.
I however also agree with you that since now we’ve a more mature version of node, it would be a great chance to rewrite those in a more clear and better way… So all your reviews and edits are welcome!

Thanks,

I am not really criticising - whenever new developers join a project (i.e. me in this case) they generally have their own opinion about things. It is this multiple developer approach that improves open source code as time goes on.

Conrad