1
\$\begingroup\$

Function getMarkets

Makes a call to get Cryptocurrency Exchange data based on asset (USD, USDC, USDT)

It calls the endpoint 3 times to return 3 arrays which are then returned to the callee.

enter image description here

enter image description here

enter image description here


I have an endpoint which hits gets exchange-markets/prices for any asset.

In my case I need to check all the paired cryptocurrency listings for USD, USDC and USDT.

At first I just tried calling getMarkets 3 times passing in the currency each time. I would then get an array of 3 responses from each call, however the last call would always replace the data for the 2 previous calls.

In order to fix that I had to use this ugly if statement indentation hell... which defeats the purpose of using async await. How should this be refactored to be cleaner, or is there a better way?

const nomicsAPI = 'https://api.nomics.com/v1/';
const nomicsKey = '8feb...;'

interface IParams {
  key: string;
  currency?: string;
}

interface IHeaders {
  baseURL: string,
  params: IParams
}

const headers: IHeaders = {
  baseURL: nomicsAPI,
  params: {
    key: nomicsKey
  }
};

const prepHeaders = (currency: string) => {
  headers.params.currency = currency;
  return axios.create(headers);
};

export const getMarkets = async (): Promise<any> => {
  try {
    let marketUSD;
    let marketUSDC;
    let marketUSDT;

    const nomicsUSD = prepHeaders('USD');
    marketUSD = await nomicsUSD.get('exchange-markets/prices');

    if (marketUSD) {
      const nomicsUSDC = prepHeaders('USDC');
      marketUSDC = await nomicsUSDC.get('exchange-markets/prices');

      if (marketUSDC) {
        const nomicsUSDT = prepHeaders('USDT');
        marketUSDT = await nomicsUSDT.get('exchange-markets/prices');

        return {
          marketUSD: marketUSD.data,
          marketUSDC: marketUSDC.data,
          marketUSDT: marketUSDT.data
        }
      }
    }
    else {
      throw new Error('USD Markets unavailable.');
    }
  } catch (err) {
    console.error(err);
  }
}

^ above produces the desired results:

enter image description here


And the actions file where this function is called from:

// Fetch USD, USDC & USDT markets to filter out Exchange List.
export const fetchMarketPrices = (asset: string) => (dispatch: any) =>
  getMarkets().then((res) => {
    console.log('res', res);

  // const combinedExchanges = res[0].data.concat(res[1].data).concat(res[2].data);
  // console.log('combinedExchanges', combinedExchanges);
  // const exchangesForAsset = combinedExchanges.filter((marketAsset: IMarketAsset) =>
  //   marketAsset.base === asset);
  // console.log('exchangesForAsset', exchangesForAsset);
  // return dispatch(actionGetMarketPrices(exchangesForAsset));
});
\$\endgroup\$
  • 1
    \$\begingroup\$ What task does this code accomplish? Please tell us, and also make that the title of the question via edit. Maybe you missed the placeholder on the title element: "State the task that your code accomplishes. Make your title distinctive.". Also from How to Ask: "State what your code does in your title, not your main concerns about it.". \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Feb 20 at 20:05
1
\$\begingroup\$

A common convention used in TypeScript/Javascript and other C-based languages is to use all uppercase letters for constants. This makes it easy to spot values that cannot be changed.

So instead of lowercase constants:

const nomicsAPI = 'https://api.nomics.com/v1/';
const nomicsKey = '8feb...;'

make them uppercase. The naming also could be improved - e.g. NOMICS_API_BASE_URL would better describe that the value is the base URL of the API than NOMICS_API.

const NOMICS_API_BASE_URL = 'https://api.nomics.com/v1/';
const NOMICS_KEY = '8feb...;'   

You could also consider using the readonly modifier with those constants.


This code (as well as the updated code in your self-answer) is quite repetitive. It would be wise to abstract the common code. That way if something needs to be updated, it can be done in one place. This adheres to the Don't Repeat Yourself (i.e. D.R.Y.) principle.

I would first recommend storing each currency in an array:

const currencies = ['USD', 'USDC', 'USDT']; 

And abstract out the common code of making the request and throwing an error if the request fails:

const getCurrencyPrice = async (currency) => {
  try {
    const request = prepHeaders(currency);
    const response =  await request.get(NOMICS_PRICES_ENDPOINT);
    if (!response) {
      throw new Error('USD Markets unavailable.');
    }
    return response.data;
  }
  catch(err) {
    console.error(err);
  }
};

That way if any of the requests fail, the Error will be thrown immediately. Also, this function has one job and could easily lend itself to unit testing.

Then use that function to iterate over the array of currencies - optionally with a for...of loop, Array.prototype.reduce(), Array.prototype.forEach(), etc

const getMarkets = async _ => {
  const returnObj = {};
  for (let currency of currencies) {
    const key = 'market' + currency;
    returnObj[key] = await getCurrencyPrice(currency);
  }
  return returnObj;
};  

Notice that the code above uses a new constant - NOMICS_PRICES_ENDPOINT - this can be defined with the other constants:

const NOMICS_PRICES_ENDPOINT = 'exchange-markets/prices';

That way if the endpoint needs to be updated, in can be done in one place. Additionally, if the constants were stored in a separate file, you wouldn't need to alter the file that contains all of this code.

\$\endgroup\$
  • \$\begingroup\$ So clean! :D thanks for the tips and design pattern. \$\endgroup\$ – Leon Gaban Feb 22 at 3:42
-1
\$\begingroup\$

Improved code:

The problem was that the headers object needed to be updated before each new await. This separated the calls into 3 different calls.

When I had the all the header objects created before the awaits they all took the last header object created.

Now the code can be re-written in a synchronous way and if statements can be avoided.

export const getMarkets = async (): Promise<any> => {
  try {
    const nomicsUSD = prepHeaders('USD');
    const marketUSD = await nomicsUSD.get(exchangeMarketPrices);
    const nomicsUSDC = prepHeaders('USDC');
    const marketUSDC = await nomicsUSDC.get(exchangeMarketPrices);
    const nomicsUSDT = prepHeaders('USDT');
    const marketUSDT = await nomicsUSDT.get(exchangeMarketPrices);

    return {
      marketUSD: marketUSD.data,
      marketUSDC: marketUSDC.data,
      marketUSDT: marketUSDT.data
    }
  } catch (err) {
    console.error(err);
  }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.