Is it appropriate to use If-Else in a function along with Fetch?

Question

I’m creating a function with a . fetch to get data from openweathermap.org, which by definition returns in Fahrenheit. Is it acceptable from the point of view of Clean Code, which says that the function should do only one thing and have few lines, make an If-Else block to control it OR is it more appropriate to make another function with a flag and return the url in the desired form and use it in requestCurrentWeather()? Below is an example of how the extensive function looks with If-Else.

let flagUnitsFormat= true;

const requestCurrentWeather = cityName => {
let URL_CURRENT_WEATHER = `http://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keyApiCurrent}`
    if(flagUnitsFormat) {
        fetch(`${URL_CURRENT_WEATHER}&units=metric`, {method:'get'})
        .then(response => {
            response.json()
            .then(result => { console.log(result)});
        })
        .catch((error) => { console.error(error); });
    } else {
        fetch(URL_CURRENT_WEATHER, {method:'get'})
        .then(response => {
            response.json()
            .then(result => { console.log(result)});
        })
        .catch((error) => { console.error(error); });
    }
}

Answer 1

No cake recipe ready. Creating a separate function just to add arguments to your URL makes your code more readable? Makes it more reusable? Try to understand the motivation to follow the rule instead of just following it blindly, maybe it doesn’t even make sense in its context.

What I see in your code that would make sense would be to avoid writing the same logic as fetch twice. This is also one of the principles of programming known as DRY. Instead of using a if/else to execute the request with or without additional arguments, I would use a logical condition only to generate the target URL. Maybe you don’t even need a logical condition.

Example:

let URL_CURRENT_WEATHER = `http://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keyApiCurrent}`
if (flagUnitsFormat) {
    URL_CURRENT_WEATHER += '&units=metric'
}

But you might want to make your role more flexible, so you can submit more arguments than units. Your function can be refactored in several ways to become more robust, if it makes sense, I would personally receive the arguments from the URL as parameter, and concatenate them into the URL

function requestCurrentWeather(cityName, args = {}) {
    let url = `http://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keyApiCurrent}`
    url += Object.keys(args).map(k => `&${k}=${args[k]}`).join('')

And of course, the function can also be async in order to be able to resolve issues in an imperative way. But if all this is in search of clean code, then the end result is always a little subjective, and it’s up to you to judge whether this is appropriate for your code

async function requestCurrentWeather(cityName, args = {}) {
    const url = `http://api.openweathermap.org/data/2.5/weather?q=${cityName}&appid=${keyApiCurrent}`
    const query = Object.keys(args).map(k => `&${k}=${args[k]}`).join('')

    const response = await fetch(url + query).then(r => r.json())
    console.log(response)
}

// exemplo de utilização:
requestCurrentWeather('curitiba', { units: 'metric' })

Answer 2

The @user204221 answer already says well about the subjective concept of how to leave the code. I wouldn’t even go into that concept so much because in your case it’s a simple if the problem.

Let’s think differently, why do you need a if? Because depending on the "flagUnitsFormat" I need to pass this in the URL.

If that’s the only answer, you could just mount the URL with a conditional, because the rest of the code is the same, you could simplify it that way:

var url = flagUnitsFormat
   ? `${URL_CURRENT_WEATHER}&units=metric`
   : URL_CURRENT_WEATHER;

fetch(url, {method:'get'})
   .then(response => {
         response.json()
   .then(result => { console.log(result)});

It is only a suggestion, but see that reinforces the idea of the other answer, depends on the point of view, but always think simpler: I need if? how do I remove this duplicate code?

Sometimes after answering that, the code will be fine :)