HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Refactor module for calls to internal image service

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
callsimagemoduleserviceforinternalrefactor

Problem

Looking for help refactoring the following code to follow a Functional Programming paradigm.

The first function builds a configuration object with default settings and optional presets, while the buildResourcePath will assemble a URL for the image service.

getSizedUrl and getSizedSkuUrl returns a URL that will be used for an image tag.

```
var imgSvcPath = './imageSizer.svc';

/**
* configureImg
* Build a configurtion object with sane defaults applied that can be used to construct a url for the ImageResizer service
*/
function configureImg( options ){
options = options || {};

var defaults = {
type: null,
src: '',
width: 120,
height: null
},
// use Image presets if an option.type provided
presets = options.type ? Images.getPresets( options.type ) : {};

return $.extend( defaults, options, presets );
};

/**
* buildResourcePath
* Construct a url for the ImageResize service to be used for the src of
*/

function buildResourcePath( svcName, options ){
// imageSizer.svc/GetResizedImage?path=[path]&size=[size]&quality=[quality]
var resourcePath = [],
conf = configureImg( options ),
src = ( conf.skuId !== undefined )
? '?sku=' + conf.skuId
: '?path=' + encodeURI( conf.src ),
size = '&size=' + ( conf.height ? 'h|' + conf.height : 'w|' + conf.width ),
quality = conf.quality ? '&quality=' + conf.quality : '';

resourcePath.push( imgSvcPath, svcName, src, size, quality );
return resourcePath.join( '' );
};

/**
* getSizedUrl
* return src of for GetResizedImage
*/
var getSizedUrl = function( options ){
if( 'undefined' === typeof options.src ) {
console.error( 'upi.Services.Image::getSizedUrl configuration options must provide an image src string' );
}

return buildResourcePath( 'GetResizedImage', options );
};

/**
* getSizedSkuUrl
* return src for an for GetResizedSkuImage
*/

Solution

// Store everything in a scope. It's to preven you from polluting the global
// scope as well as protect your code from collisions.
;(function(ns) {

  // Pulling out constants into this scope. They only need to be declared once.
  var imgSvcPath = './imageSizer.svc';
  var defaults = {
    type: null,
    src: '',
    width: 120,
    height: null
  };

  function configureImg(options) {
    var options = options || {};
    var presets = options.type ? Images.getPresets(options.type) : {};

    // It's best if you extend to a blank object. Don't override the defaults.
    return $.extend({}, defaults, options, presets);
  }

  function buildResourcePath(svcName, options) {

    var conf = configureImg(options);

    // I assume you use jQuery because of $.extend so I introduce $.param
    // which takes a map and converts it into a serialized parameter list

    var params = {};

    // And ternaries can be very messy and unreadable. Now using if-else instead.
    if (conf.skuId !== undefined) params.sku = conf.skuId;
    else params.path = conf.src;

    if (conf.height) params.size = 'h|' + conf.height;
    else params.size = 'w|' + conf.width;

    if (conf.quality) params.quality = conf.quality;

    return imgSvcPath + '?' + $.param(params);
  }

  // Let's append our API to the namespace

  ns.getSizedUrl = function(options) {
    if ('undefined' === typeof options.src) {
      console.error('upi.Services.Image::getSizedUrl configuration options must provide an image src string')
    }
    return buildResourcePath('GetResizedImage', options)
  };

  ns.getSizedUrlForSku = function(skuId, options) {
    if ('undefined' === typeof skuId) {
      console.error('upi.Services.Image::getSizedSkuUrl configuration options must provide sku ID')
    }
    options = options || {};
    options['skuId'] = skuId;
    return buildResourcePath('GetResizedSkuImage', options)
  };

  // Change your namespace to something else. Image is already taken.
  // Image is the constructor for making image elements with JS.
}(this.MyImage = this.MyImage || {}))

Code Snippets

// Store everything in a scope. It's to preven you from polluting the global
// scope as well as protect your code from collisions.
;(function(ns) {

  // Pulling out constants into this scope. They only need to be declared once.
  var imgSvcPath = './imageSizer.svc';
  var defaults = {
    type: null,
    src: '',
    width: 120,
    height: null
  };

  function configureImg(options) {
    var options = options || {};
    var presets = options.type ? Images.getPresets(options.type) : {};

    // It's best if you extend to a blank object. Don't override the defaults.
    return $.extend({}, defaults, options, presets);
  }

  function buildResourcePath(svcName, options) {

    var conf = configureImg(options);

    // I assume you use jQuery because of $.extend so I introduce $.param
    // which takes a map and converts it into a serialized parameter list

    var params = {};

    // And ternaries can be very messy and unreadable. Now using if-else instead.
    if (conf.skuId !== undefined) params.sku = conf.skuId;
    else params.path = conf.src;

    if (conf.height) params.size = 'h|' + conf.height;
    else params.size = 'w|' + conf.width;

    if (conf.quality) params.quality = conf.quality;

    return imgSvcPath + '?' + $.param(params);
  }

  // Let's append our API to the namespace

  ns.getSizedUrl = function(options) {
    if ('undefined' === typeof options.src) {
      console.error('upi.Services.Image::getSizedUrl configuration options must provide an image src string')
    }
    return buildResourcePath('GetResizedImage', options)
  };

  ns.getSizedUrlForSku = function(skuId, options) {
    if ('undefined' === typeof skuId) {
      console.error('upi.Services.Image::getSizedSkuUrl configuration options must provide sku ID')
    }
    options = options || {};
    options['skuId'] = skuId;
    return buildResourcePath('GetResizedSkuImage', options)
  };

  // Change your namespace to something else. Image is already taken.
  // Image is the constructor for making image elements with JS.
}(this.MyImage = this.MyImage || {}))

Context

StackExchange Code Review Q#47918, answer score: 5

Revisions (0)

No revisions yet.