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

Question

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')

Answer 1

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.

Answer 2

There is nothing wrong with what you've done, but you could also create 3 more functions to call that in turn call getProductsByCurrency with the respective type.

var example = JSON.parse(`[{"id":1,"name":"1","price":5},{"id":2,"name":"2","price":15},{"id":3,"name":"3","price":20}]`);

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

function getPhonesByCurrency() {
  return getProductsByCurrency("phone", ...arguments);
}

function getCosmeticsByCurrency() {
  return getProductsByCurrency("cosmetic", ...arguments);
}

function getBooksByCurrency() {
  return getProductsByCurrency("book", ...arguments);
}

console.log([].concat(getPhonesByCurrency(example), getCosmeticsByCurrency(example, 0.5), getCosmeticsByCurrency(example, 2)));

You might also prefer wrapping those 3 functions in an object (more tidy and helps IDE with autocomplete)

var example = JSON.parse(`[{"id":1,"name":"1","price":5},{"id":2,"name":"2","price":15},{"id":3,"name":"3","price":20}]`);

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

const getByCurrency = {
  phones: function() {
    return getProductsByCurrency("phone", ...arguments);
  },
  cosmetics: function() {
    return getProductsByCurrency("cosmetic", ...arguments);
  },
  books: function() {
    return getProductsByCurrency("book", ...arguments);
  }
};

console.log([].concat(getByCurrency.phones(example), getByCurrency.cosmetics(example, 0.5), getByCurrency.books(example, 2)));

Answer 3

that depends on the kind of data you're feeding into these functions. if you'll pass different arrays of objects (that all have a type property) than I reckon you'd have a function that filters arrays by type ( or any other property and condition that is common between different sets of data). You could than chain your filter function with your mapping function. Your mapping function seems that it needs to be specific to the currenncy as you're plucking certain props from the object, than computing some of the values before you return it.

Hope this helps