Recreating a URL string

Question

This is what my viewObject looks like:

{
 description: "GOOG AAPL TSLA"
 id: 42
 ticker_1: "GOOG"
 ticker_2: "AAPL"
 ticker_3: "TSLA"
 timespan: "1mo"
}

Here is the function use to check the object. In a very dirty fashion, it creates a string to be used to replace the URL with:

function createUrl(viewObj) {
 var urlString = '',
 ticker1 = '',
 ticker2 = '',
 ticker3 = '',
 time = '';
 if (viewObj.ticker_1) {
 ticker1 = '&ticker_1='+viewObj.ticker_1;
 ulrString = ulrString+ticker1;
 }
 if (viewObj.ticker_2) {
 ticker2 = '&ticker_2='+viewObj.ticker_2;
 ulrString = ulrString+ticker2;
 }
 if (viewObj.ticker_3) {
 ticker3 = '&ticker_3='+viewObj.ticker_3;
 ulrString = ulrString+ticker3;
 }
 if (viewObj.timespan) {
 time = '&timespan='+viewObj.timespan;
 ulrString = ulrString+time;
 }
 console.log('/dashboard'+ulrString);
 return '/dashboard'+ulrString;
}

I have multiple if/else statements and code that can be repeated. How would you accomplish this?

• Roman Susi: Have you taken a look at stackoverflow.com/questions/316781/… ?
• Leon Gaban: Thanks for the link, however that solution is definitely not what I want... hmm I will see if there is a lodash solution for this
• Dan: Is this in a Node environment or in the browser?
• Leon Gaban: Browser, this is a small function in my Angular app
• Dan: Any reason you aren't using the params options in $http for this? $http.get('/dashboard', { ticker_1: ticker_1 }) would resolve to /dashboard?ticker_1=foo if ticker_1 is foo or just /dashboard otherwise. It's also a service - $httpParamsSerializer

Answer 1

You can try this:

function createUrl(viewObj) {
 var urlParams = ['ticker_1', 'ticker_2', 'ticker_3', 'timespan'],
 urlString = '',
 current;
 for (var i = 0; i < urlParams.length; i++) {
 current = urlParams[i];
 if (viewObj[current]) urlString += '&' + current + '=' + viewObj[current];
 }
 console.log('/dashboard' + urlString);
 return '/dashboard' + urlString;
}

The algorithm explained:

As the treatment you need for each of the URL parameters is the same, it's enough with storing all of them in an array (urlParams) and then iterating over each that array.

At each iteration, current will be one of them. E.g.: at the first iteration, current will be 'ticker_1'. That way,

if (viewObj[current])

replaces

if (viewObj.ticker_1)

Besides, it's not necessary to store the new string to a variable (as you do with ticker1, ticker2, etc.), before appending it to urlString, so I just removed those lines.

Finally, for string concatenation, the += operator can be used, so

urlString = urlString + something;

can be replaced with

urlString += something;

• Leon Gaban: Thanks! I love how clean and simple this is, I get it too... I know what params to look for, I set current to the next param it finds, if it finds it add it to the string. The loop does however has to run for each possible param, I wonder if there is a way to optimize that? Maybe a _.find or something with lodash?

Answer 2

Do you mean write code that would repeat the multiple if/else statements? cause that would be the right idea and make your code more DRY.

The first thing to take a look at is your data structure. As it is, you have hard-coded a handful of ticker properties that would make iteration difficult. You could use a for/in loop to go through the properties of your object, but we know we don't want the description or id in the string. Better to put all tickers in an array. That way it'll be easy to create a string with as many (or little) tickers as you want.

{
 description: "GOOG AAPL TSLA"
 id: 42
 tickers: ["GOOG", "AAPL", "TSLA"]
 timespan: "1mo"
}

It's now easy to iterate over the tickers and construct a string out of them. Since we always start with ticker_<n> as the beginning, we can construct that inside our loop, using the index to count which ticker # we are on.

For the function:

function createUrl(viewObject){
 // use .map to add "ticker_<n>=<value>" to each of the tickers,
 // then join with "&" for the query.
 var urlString = viewObject.tickers
 .map(function(ticker, index){
 // prepend the first query with its own '&'
 var and = index === 0 ? "&" : "";
 //create a 1-indexed count of the tickers.
 var tickerNum = index + 1; 
 return and + "ticker_" + tickerNum + "=" + ticker;
 })
 .join('&'); //<--separate each query with "&"
 // for timespan, you don't need to make a seperate `time` variable,
 // this can be done in one line.
 if(viewObject.timespan) urlString += "&timespan=" + viewObject.timespan;
 console.log('/dashboard'+urlString);
 return '/dashboard'+urlString;
}

• SirPython: Welcome to Code Review! Good job on your first answer.