If a function is too long, is it recommended to divide it into smaller ones?

Question

I have the following function, which I intend to extend:

def get_dataframe(site, search):
    if site == 'pichau':
        try:
            soup = get_page(f'https://www.pchau.com.br/catalogsearch/result/?q={search}')
            assert soup

            price_cartao = [i.getText() for i in soup.select('.other .valor')]
            price_boleto = [i.getText().replace('à vista', '') for i in soup.select('.boleto .valor')]
            product = [i.get('title').upper() for i in soup.select('.title a')][:len(price_cartao)]

            data = {'Produto': product, 'Preço à vista': price_boleto, 'Preço cartão': price_cartao}
            product_df = DataFrame(data)
            return product_df

        except Exception as e:
            return 'Problema com a conexão entre o site!'

    elif site == 'chipart':
        try:
            soup = get_page(f'https://www.chipart.com.br/produtos/{search}')
            assert soup

            price_boleto = [i.getText() for i in soup.select('.products__list__item .product-card__price__final .price')]
            price_cartao1 = [i.getText().strip().replace('\t', '').replace('\n', '').replace('em', '') for i in soup.select('.products__list__item .installments')]
            price_cartao2 = [i.getText() for i in soup.select('.products__list__item .price-installments .price')]
            product = [i.getText().strip().replace('\t', '').replace('\n', '') for i in soup.select('.product-card__title .title')][:len(price_boleto)]
            price_cartao = [f'{price_cartao1[i]} de {price_cartao2[i]}' for i in range(len(price_boleto))]

            data = {'Produto': product, 'Preço à vista': price_boleto, 'Preço cartão': price_cartao}
            product_df = DataFrame(data)
            return product_df

        except Exception as e:
            return 'Problema com a conexão entre o site!'

In this case, the recommendable would be to split into several ifs "break it" into several functions?

Answer 1

Separate them into modules. The good part of python is that you can only instantiate them in import.

Depending on the case, extend the parameters and reuse a function for different tasks.

Answer 2

Hello,

split a function or not is not always tied to its size, but usually to the processing steps it has.

In your example we clearly have repetitive code and that can be separated into smaller specialized functions.

Follows a simplified proposal for modification:

def get_dataframe(site, search):
    try:
        if site == 'pichau':
            product, price_boleto, price_cartao = pichau(search)
        elif site == 'chipart':
            product, price_boleto, price_cartao = chipart(search)
        else:
            # Se for possivel ter outro valor para site

        data = {'Produto': product, 'Preço à vista': price_boleto, 'Preço cartão': price_cartao}
        product_df = DataFrame(data)
        return product_df

    except Exception as e:
        return 'Problema com a conexão entre o site!'

def pichau(search):
    soup = get_page(f'https://www.pchau.com.br/catalogsearch/result/?q={search}')
    assert soup

    price_cartao = [i.getText() for i in soup.select('.other .valor')]
    price_boleto = [i.getText().replace('à vista', '') for i in soup.select('.boleto .valor')]
    product = [i.get('title').upper() for i in soup.select('.title a')][:len(price_cartao)]

    return product, price_boleto, price_cartao

def chipart(search):
    soup = get_page(f'https://www.chipart.com.br/produtos/{search}')
    assert soup

    price_boleto = [i.getText() for i in soup.select('.products__list__item .product-card__price__final .price')]
    price_cartao1 = [i.getText().strip().replace('\t', '').replace('\n', '').replace('em', '') for i in soup.select('.products__list__item .installments')]
    price_cartao2 = [i.getText() for i in soup.select('.products__list__item .price-installments .price')]
    product = [i.getText().strip().replace('\t', '').replace('\n', '') for i in soup.select('.product-card__title .title')][:len(price_boleto)]
    price_cartao = [f'{price_cartao1[i]} de {price_cartao2[i]}' for i in range(len(price_boleto))]

    return product, price_boleto, price_cartao

Note that the function get_dataframe made it much simpler and readable. Which, depending on the size of the project, makes it infinitely easier to maintain and improve.

The part of the return of the sub-functions pichau and chipart is at your discretion. Here I preferred to return the separate objects to avoid code repetition. If you prefer, you can already return a dataframe or dictionary.