DEV Community

Discussion on: The case for DRY code

Collapse
 
grahamthedev profile image
GrahamTheDev • Edited

This example needs a little bit of a rethink as at the moment you are comparing apples and oranges.

The first function searches one field whereas the second searches all fields.

Secondly the second function returns partial matches which may not be desirable.

Lets say I wanted to search for only properties in the state "rivers", in the first example I would do findOrdersByState ("rivers"); and get the expected result.

In the second example I might have a town called "tranquil rivers" and it would also be returned.

Finally the first code would be far more performant as you aren't having to loop through each object parameter.

Perhaps the following would be a better example (untested)?

function findOrderByAddressField(value, field) {
    field = field || "state";
    let foundOrders = []
    orders.forEach(order => {
        if (order.address[field] == value) {
            foundOrders.push(order)
        }
    })
    return foundOrders
}

const riversOrders = findOrderByAddressField('rivers', 'state') // find order by state
const phOrders = findOrderByAddressField('port harcourt', 'city') // find orders by city
const NigerianOrders = findOrderByAddressField('nigeria', 'country') // find orders by country
const riversOrders2 = findOrderByAddressField('rivers') // find orders by state by default
Enter fullscreen mode Exit fullscreen mode

Obviously I had to add a check for a null "field" otherwise it could throw an error, the advantage is it now has a default if you want (if you didn't want this behaviour you could just return false if it wasn't set).

Obviously it would still throw an error if an invalid field was entered, so at that point you might also want to check that depending on what behaviour you want. (i.e. findOrderByAddressField('rivers', 'area') would throw an error);

Collapse
 
kalashin1 profile image
Kinanee Samson • Edited

I see, this wouldn't be the final solution, and if indexOf returns partial matches, you can use array.find() instead.

function findOrderByAddressField(field) {
    let foundOrders = []
    let foundOrder;
    orders.forEach(order => {
      foundOrder = Object.values(order.address).find(v => v === field)
     foundOrders.push(foundOrder)
    })
    return foundOrders
}

const riversOrders = findOrderByAddressField('rivers') // find order by state
const phOrders = findOrderByAddressField('port harcourt') // find orders by city
const NigerianOrders = findOrderByAddressField('nigeria') // find orders by country
Enter fullscreen mode Exit fullscreen mode
Collapse
 
grahamthedev profile image
GrahamTheDev • Edited

You still have the issue of matching across all fields in the above example, so a search for "rivers" would match on state, city and country.

Collisions would be rare but if this was something else like names with first name and surname fields, you could search for "Frank" and it would match "Frank Johnson" and "John Frank".

The code I wrote was to mirror your original functions where each one was for a specific field but your function is ideal for if you want to search across all 3 fields at once, it just doesn't do the same as the first functions so isn't ideal as a comparison.

Obviously it all depends on what the end use case is!