Search code examples
javascriptarraysfunctional-programmingreusabilitypartial-application

How to make this function more reusable/specific/better design?


I wrote this function below which transforms the passed array of products by product type and currency type

function getProductsByCurrency(products, type, exchangeRate = 1) {
    var productsRetrieved = products.map(item => ({id: item.id,
        name: item.name,
        price: (item.price * exchangeRate).toFixed(2),
        type: type}));
    return productsRetrieved;       
}

Is it possible to break down the function to be more specific? or design it in a better way? For example by naming it getProductsByCurrency it doesn't look right because if I am using it with default rate, i can pass books array to retrieve products with 'books' type independent of exchange rate. Perhaps there is a way to use partial functions(FP)?

Edit: Adding more context to what I am trying to achieve.

Lets say I have three categories of products(phones, cosmetics, books) coming from three resources. I need to create three consolidated arrays of all the products by different currencies(productsinUSD, productsinAUD, productsinPounds)

Also using below function to consolidate the arrays

    function concatProducts(arr) {
        return [].concat.apply([], arr);
    }

So I am calling getProductsByCurrency three times to transform them by product type and currency(exchange rate) and passing those values as an array to concat them to achieve productsinUSD. And repeat to get productsinAUD, productsinPounds.

Also type is string value(ex: 'mobiles')


Solution

  • First there's nothing really wrong with the function you posted. There's a few things I'd do differently, but I'm not going to pretend this isn't splitting hairs a bit.

    const processItem = (type, exchangeRate = 1) => ({
      id,
      price,
      name,
    }) => ({
      id,
      name,
      type,
      price: (price * exchangeRate).toFixed(2),
    });
    

    We have a function that takes a type and an optional exchangeRate and returns a function that converts a single item to the form you want. This is what bob was talking about in the comments. I've also used object destructuring on the item and property shorthand on the result to make the code cleaner. Now we can map it over various categories of stuff:

    const results = [
       ...mobilePhones.map(processItem('phone')),
       ...cosmetics.map(processItem('cosmetics')),
       ...books.map(processItem('book')),
    ];
    

    If you need the interim results for some other purpose, just stuff them in vars but I've spread them directly into the results array for simplicity.

    While this is shorter, clearer, and more flexible than the one you posted for the code quality hat trick, I'd like to reiterate that I've seen way, way worse than the function you posted.