Coding style

[hansmbakker]

Personally I find the coding style in iotivity-node hard to follow, for example because of the nesting combined with ternary operators ( ?: ). Long functions that are defined inline as properties are also an example.

Improving this would make it easier to find bugs or understand what is happening. Is this something you'd be open to?

Example from https://github.com/otcshare/iotivity-node/blob/master/lib/Client.js#L344:

	retrieve: function( resourceId, query, listener ) {

		// Argument juggling
		// If @query is a function, then the query is not specified, but a listener is, so we
		// shift the arguments around and initialize @query
		if ( arguments.length === 2 && typeof query === "function" ) {
			listener = query;
			query = undefined;
		}

		// We must first ensure we have a valid ClientResource object
		return ( ( resourceId instanceof ClientResource ) ?

			// If resourceId is a ClientResource object we can use it as such, but only if the
			// options given are the same as the options assigned to it. Otherwise we must create
			// and return a new ClientResource object.
			Promise.resolve( _.isEqual( resourceId._private.query, query ) ?
				resourceId : ClientResource( resourceId, true ) ) :

			// Otherwise, we must discover the resource first, because we need to establish its
			// types and interfaces before we can perform the retrieve().
			client.findResources( {
				deviceId: resourceId.deviceId,
				resourcePath: resourceId.resourcePath,
				_resolve: true
			} ) ).then( function( resource ) {
				var get = function() {
					return oneShotRequest( {
						prefix: "retrieve",
						method: "OC_REST_GET",
						requestUri: resource.resourcePath,
						query: query,
						deviceId: resource.deviceId,
						createAnswer: function( response ) {
							var properties = response.payload ?
								payload.repPayloadToObject( response.payload ) : {};
							if ( properties instanceof Error ) {
								return properties;
							} else {
								_.extend( resource.properties, properties );
								return resource;
							}
						}
					} );
				};

				// Make sure the requested query (if any) is associated with this resource
				if ( !( "query" in resource._private ) && query ) {
					resource._private.query = query;
				}

				return listener ?
					new Promise( function( fulfill, reject ) {
						var result = ClientResource.observe( resource, fulfill, reject );

						if ( !( result instanceof Error ) ) {
							resource.on( "update", listener );
						}

						// When ClientResource.observe() returns true it means it is unable to
						// resolve the promise, which in turn means that we must perform a get().
						// So, let's resolve this promise with a special value (true), so that the
						// chained promise can call get().
						if ( result ) {
							fulfill( true );
						}
					} ).then( function( result ) {
						return ( result === true ? get() : result );
					} ) :
					get();
			} ).catch( function( error ) {
				return Promise.reject( error );
			} );
	}

[gabrielschulhof]

I guess you can turn this into a sequence of ``` if ( something ) { return x; } else if ( somethingElse ) { return y; } else { return z; }

…

On Mon, Feb 13, 2017 at 10:31 PM, wind-rider ***@***.***> wrote: Personally I find the coding style in iotivity-node hard to follow, for example because of the nesting combined with ternary operators ( ?: ). Improving this would make it easier to find bugs or understand what is happening. Is this something you'd be open to? Example from https://github.com/otcshare/iotivity-node/blob/master/lib/ Client.js#L344: retrieve: function( resourceId, query, listener ) { // Argument juggling // If @query is a function, then the query is not specified, but a listener is, so we // shift the arguments around and initialize @query if ( arguments.length === 2 && typeof query === "function" ) { listener = query; query = undefined; } // We must first ensure we have a valid ClientResource object return ( ( resourceId instanceof ClientResource ) ? // If resourceId is a ClientResource object we can use it as such, but only if the // options given are the same as the options assigned to it. Otherwise we must create // and return a new ClientResource object. Promise.resolve( _.isEqual( resourceId._private.query, query ) ? resourceId : ClientResource( resourceId, true ) ) : // Otherwise, we must discover the resource first, because we need to establish its // types and interfaces before we can perform the retrieve(). client.findResources( { deviceId: resourceId.deviceId, resourcePath: resourceId.resourcePath, _resolve: true } ) ).then( function( resource ) { var get = function() { return oneShotRequest( { prefix: "retrieve", method: "OC_REST_GET", requestUri: resource.resourcePath, query: query, deviceId: resource.deviceId, createAnswer: function( response ) { var properties = response.payload ? payload.repPayloadToObject( response.payload ) : {}; if ( properties instanceof Error ) { return properties; } else { _.extend( resource.properties, properties ); return resource; } } } ); }; // Make sure the requested query (if any) is associated with this resource if ( !( "query" in resource._private ) && query ) { resource._private.query = query; } return listener ? new Promise( function( fulfill, reject ) { var result = ClientResource.observe( resource, fulfill, reject ); if ( !( result instanceof Error ) ) { resource.on( "update", listener ); } // When ClientResource.observe() returns true it means it is unable to // resolve the promise, which in turn means that we must perform a get(). // So, let's resolve this promise with a special value (true), so that the // chained promise can call get(). if ( result ) { fulfill( true ); } } ).then( function( result ) { return ( result === true ? get() : result ); } ) : get(); } ).catch( function( error ) { return Promise.reject( error ); } ); } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#123>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7k0ctFNiIjIRAIoMJzLISABcEme4WCks5rcL2KgaJpZM4L_r3m> .