Emily

Emily

My First Elixir Module: How Can I Make This Code Cleaner?

Well, after the mess my last project in Python ended up being, I’m committed this time to make the cleanest code possible.

Before I get any bad habits, I’d appreciate a second set of eyes on the code below.

If you see any bad directions I’m going down, or ways to make the code cleaner, I’d really appreciate being set on the right track now.

(See code in full context below.)

How would I make this code cleaner? (Pipeline operator? Recursion maybe?)

def calc_pyramid_prices_percents(prices, initial_percent) do

    total_column_percent = calc_total_column_percent(prices, initial_percent)
    total_pyramid_percent = calc_total_pyramid_percent(total_column_percent)
    [ initial_price | _ ] = prices
    total_distance = calc_total_distance(prices, initial_price)

    Enum.map(prices, fn(price) -> calc_price_percent(price, initial_price, \
        total_distance, initial_percent, total_pyramid_percent) end)
end

Is there a cleaner way to do this?

def calc_max_initial_percent(prices) do
    inverted_prices = Enum.reverse(prices)
    [d] = Enum.take((calc_pyramid_prices_percents(inverted_prices, 0)), -1) # get last price percent.
    d # Return only the number.
end

Do you see any red flags or better coding practices that should have been followed in the code in full context?

defmodule PyramidCalculator do
    @moduledoc """
    Calculates a price percents adding up to 100 that form a pyramid shape
    based on the relative distance of each price point from the initial price point.

    If initial percent is greater then number of prices / 100, an inverted pyramid is returned.

    ## IMPORTANT
    The first price in the price list must be the highest or lowest price in the list.
    List is returned unsorted.  Sort list ascending or descending order before invoking the pyramid calculator.
    """

    @doc """
    Returns a list of percents for each price point that would look like a pyramid
    based on the relative distance of each price point from the initial price point.
    
    ## Examples:

        ## Normal pyramid example:
        iex> prices = [1, 2, 3]
        iex> initial_percent = 10
        iex> PyramidCalculator.calc_pyramid_prices_percents(prices, initial_percent)
        [10.0, 33.33333333333333, 56.666666666666664]

        ## Inverted pyramid example:
        iex> prices = [1, 2, 3]
        iex> initial_percent = 40
        iex> PyramidCalculator.calc_pyramid_prices_percents(prices, initial_percent)
        [40.0, 33.333333333333336, 26.666666666666668]

        ## Asymentrical pyramid price point distances example:
        iex> prices = [1, 3, 7, 22]
        iex> initial_percent = 5
        iex> PyramidCalculator.calc_pyramid_prices_percents(prices, initial_percent)
        [5.0, 10.517241379310345, 21.551724137931036, 62.93103448275862]

    """
    def calc_pyramid_prices_percents(prices, initial_percent) do

        total_column_percent = calc_total_column_percent(prices, initial_percent)
        total_pyramid_percent = calc_total_pyramid_percent(total_column_percent)
        [ initial_price | _ ] = prices
        total_distance = calc_total_distance(prices, initial_price)

        Enum.map(prices, fn(price) -> calc_price_percent(price, initial_price, \
            total_distance, initial_percent, total_pyramid_percent) end)
    end

    @doc """
    Calculates the maximum the initial percent can be so that the final
    price point percent in the prices list is zero.

    ## Examples:

        iex> prices = [1, 2, 3]
        iex> PyramidCalculator.calc_max_initial_percent(prices)
        66.66666666666666

        iex> prices = [5, 10, 15, 20]
        iex> PyramidCalculator.calc_max_initial_percent(prices)
        50.0
    """
    def calc_max_initial_percent(prices) do
        inverted_prices = Enum.reverse(prices)
        [d] = Enum.take((calc_pyramid_prices_percents(inverted_prices, 0)), -1) # get last price percent.
        d # Return only the number.
    end

    @doc """
    Calculates a perfect column-shaped price percents based on number of price points.

    ## Examples:

        iex> prices = [1, 2, 3]
        iex> PyramidCalculator.calc_column_percent(prices)
        33.333333333333336

        iex> prices = [5, 11, 25, 44]
        iex> PyramidCalculator.calc_column_percent(prices)
        25.0
    """
    def calc_column_percent(prices), do: 100 / Enum.count(prices)

    defp calc_total_column_percent(prices, initial_percent), do: Enum.count(prices) * initial_percent
    
    # In case of an inverted pyramid, returns a negative total.
    # Which means pyramid percent will be subtracted from the
    # total column percent rather then added, on the final 
    # price percent calculation.
    defp calc_total_pyramid_percent(total_column_percent), do: 100 - total_column_percent
    
    defp calc_price_percent(price, initial_price, total_distance, \
        initial_percent, total_pyramid_percent) do
     
        ratio = calc_price_pyramid_ratio(initial_price, price, total_distance)  
        price_pyramid_percent = calc_price_pyramid_percent(ratio, total_pyramid_percent)

        initial_percent + price_pyramid_percent
    end

    defp calc_price_pyramid_ratio(initial_price, price, total_distance),  \
        do: abs(price - initial_price) / total_distance

    defp calc_price_pyramid_percent(ratio, total_pyramid_percent), do: ratio * total_pyramid_percent

    defp calc_total_distance(prices, initial_price), do: _sum_distance(prices, initial_price, 0)
    
    defp _sum_distance([], _initial_price, total_distance), do: total_distance
        
    defp _sum_distance( [ price_head | price_tail ], initial_price, total_distance) do
        _sum_distance(price_tail, initial_price, (total_distance + abs(price_head - initial_price)))
    end
end

Most Liked

JEG2

JEG2

Author of Designing Elixir Systems with OTP

I just felt compelled to point out that this is a life long mission. It’s often a worthy road to travel, but don’t be frustrated if you never arrive. The journey’s the important bit.

Also, there are sometimes good reasons to sacrifice clean code. Try not to get to hung up on the path to purity.

:slight_smile:

bbense

bbense

You’ve written a perfectly straightforward translation of a typical procedural language module into Elixir. So as far as that goes I can’t find much to “fix” in the module.

However, there’s a kind of larger problem at the architectural level. The way that I find most useful to design Elixir programs is to think of data and the transformations required on that data. What data needs to be “together” to flow through the program? Where can I cleanly split the data groupings into smaller data groups with their own local transformations?

One of the outcomes of thinking this way is to have modules with a specified purpose and function signatures that follow the pattern of

def transformation( thing_to_transform, data_needed_to_specify_the_transform) 

Function signatures ( or arities ) much higher than 3 or 4 or lots of foo/3, foo/4 functions in a module are a warning sign that you might need to re-think how you’re constructing your code.[1] Without the larger context of the application which uses your module, it’s hard to say for sure, but to me it really feels like there is an abstraction one level above that is needed to clean up this code. Or it could just be me overthinking it, with too much coffee and too much time waiting for S3 to sync.

This is a great post about design in Elixir.

Latest Erlangist Blog Post

One minor last point, you don’t need to start private functions with an underscore. Since
the underscore has special meaning on variable names, I’d encourage you not to use
it elsewhere.

[1]- Or it might mean you’re writing some truly generic functionality like GenServer.

globalkeith

globalkeith

Wise words!

globalkeith

globalkeith

As mentioned, its a personal preference. I find the general abstraction of “PyramidCalculator” gives enough context that the thing is a calculator…

bbense

bbense

Don’t worry too much, I am often very wrong. It’s my second superpower[1]. Allowing yourself to be wrong lets you put stuff out there and learn. Don’t take anything personal, you are not your code.

[1]- The first is reading man pages.

Where Next?

Popular in Questions Top

dotdotdotPaul
Okay, I'm having a heck of a time trying to figure out how to best handle the validation of belongs_to associations in Ecto. I'm sure I'...
New
pmjoe
I have a relationship of love and hate with Elixir. Lots of things are just absolutely right, but there are some things that are kind of ...
New
quazar
How to set Jason to encode all fields in ecto schema, I don’t care about security and implementing only is taking long list of attributes...
New
rms.mrcs
Hi, I need to transform a list of numbers into a map where the keys are the indexes and the values are the original values of the list....
New
stefanluptak
Hello everybody, usually, I use a 29" ultra-wide monitor for VSCode which can easily accomodate explorer (files panel) + file with code ...
New
electic
Hi, I am new to Elixir. I am trying to use the DateTime component to insert a date into MySQL however the there seems to be no way to fo...
New
Codball
Mix format works fine if run from the cmd. I’ve followed this to facilitate the implementation into VSC which involves downloading an ext...
New
romenigld
I am trying to run a deploy with docker and I successfully runned with this command: docker build -t romenigld/blog-prod . but when I t...
New
siddhant3030
Hi, I have to write a raw query for one of my project. But till now I have used ecto queries and don’t have much experience writing raw ...
New
jay1
Why is it that the mnesia database isn’t the most preferred database for use in Elixir/Phoenix?
New

Other popular topics Top

albydarned
Hello all! I am typing this post from my new MacBook Pro with the M1 chip. I’m loving it so far, and will probably use it as my daily dr...
New
sergio_101
I am VERY much an elixir newbie. I have taken one elixir course and one phoenix course on Udemy. During that course, I saw the instructor...
New
_russellb
I want to try my hand at web scraping. What tools/libraries do I need to use. I’m hoping to turn this into something professional so don’...
New
New
alice
Hey, Just curious what are the main benefits of Elixir compared to Clojure? When is Elixir more useful than Clojure and vice versa? Th...
New
fireproofsocks
Forgive me if this is obvious, but how does one delete a database record WITHOUT selecting it first? https://hexdocs.pm/ecto/Ecto.Repo.h...
New
WestKeys
Currently suffering from paralysis by [HTTP client] analysis. This is rather unusual in Elixirland as there tends to be consensus on the ...
New
belgoros
I’m not a pro in using Regex and can’t figure out why the following behaviour happens, especially if we take into account the difference ...
New
skosch
To my knowledge, put_in, Map.update etc. all have the one limitation of not automatically creating intermediate keys when needed (for exa...
New
magnetic
Hey :wave:t3: Elixir community, I’ve been learning Elixir, and working on some side projects. My editor of choice is VSCode, and althoug...
New

We're in Beta

About us Mission Statement