Could use some help making my code more elixir-like with respect to style and efficiency

I’ve been working through the books, answering the questions and building my own v.small projects, it has gotten to a point where I am writing code to get the problem solved so I can progress to the next topics, and even if I expand on the material for extra knowledge, it looks like I might not be fully utilising elixirs conventions.

Example

  #all?

  def all?([], _), do: true

  def all?([h|t], f), do: process_all([h|t], f, f.(h))

  def process_all([], _, b) when b == true, do: b

  def process_all([h|t], f, b) when b == true, do: process_all(t, f, f.(h))

  def process_all(_, _, b) when b == false, do: b

The above seems sort clunky compared to my vague recollection of some examples in prior chapters. What sort of procedure should I follow when choosing a style and also determining the best structure for efficient memory use.

I am currently looking back over previous chapters so that I can see where the disparity lay, but would appreciate some pointers. Below are perhaps a more egregious examples:

 #split

  def split([], _), do: {[], []}

  def split(e, 0), do: {[], e}

  def split(e, count) do

    prep_c = fn

      (_, _, _, count) when count > 0 -> count

      (_, [], acc, neg_c) ->
        if acc < -neg_c do
          0
        else
          acc + neg_c
        end

      (f, [_|t], acc, neg_c) -> f.(f, t, acc + 1, neg_c)

    end

    process_split(e, prep_c.(prep_c, e, 0, count), {[], []})

  end

  def process_split([], _, {l_i, l_ii}), do: {reverse(l_i, []), reverse(l_ii, [])}

  def process_split([h|t], count, {l_ac, list}) when count > 0, do: process_split(t, count - 1, {[h|l_ac], list})

  def process_split([h|t], c, {list, l_ac}) when c === 0, do: process_split(t, 0, {list, [h|l_ac]})

  ##Reverse

  def reverse([], acc), do: acc

  def reverse([h|t], acc), do: reverse(t, [h|acc])

  #Flatten

  def flatten([]), do: []

  def flatten(list), do: process_flat([], list)

  # def flaten(p), do: raise "Argument expected list got #{IO.inspect p}"

  def list?([_|_]), do: true

  def list?([]), do: true

  def list?(_), do: false

  def process_flat(list, []), do: reverse(list, [])

  def process_flat(list, [h|t]) do
    cond do
      !list?(h) ->
        process_flat([h|list], t)
      h !== [] ->
        [h_1|t_1] = h
        t_2 = [t_1|t]
        process_flat(list, [h_1|t_2])
      true ->
        process_flat(list, t)
    end
  end

For one you can just pattern match on the bools:

  def all?([], _), do: true

  def all?([h|t], f), do: process_all([h|t], f, f.(h))

  def process_all([], _, true), do: true

  def process_all([h|t], f, true), do: process_all(t, f, f.(h))

  def process_all(_, _, false), do: false
2 Likes

Lovely, I don’t know how I didn’t see that. It looks much leaner.

I really need to change the way I think about these.

1 Like

fyi in the second and third implementations it was required that I do not use library functions, hence why I do my own list checking etc.

First thing that jumps out is this head:

def all?([h|t], f), do: process_all([h|t], f, f.(h))

Putting a list back together after you’ve pattern-matched it apart should give you that “clunky” vibe.

Second thing is this head:

def process_all([h|t], f, b) when b == true, do: process_all(t, f, f.(h))

The first call of process_all is going to already have f.(h) in b. Calling the supplied function twice for some parts of the input should also give you that “clunky” vibe.

Here are some alternative shapes for that function:

  def all?([], _), do: true

  def all?([h|t], f) do
    if f.(h) do
      all?(t, f)
    else
      false
    end
  end

This version ensures that f.(h) is called at most once for each h. However, you still might get a strange vibe from an if with a constant boolean value in else.

That’s because there’s a shorter version that uses an Elixir operator to encapsulate that pattern:

  def all?([], _), do: true

  def all?([h|t], f) do
    f.(h) and all?(t, f)
  end

This version has a very small behavioral difference: f is now required to return a boolean, whereas the if version only looks for “not false or nil”.

5 Likes

Replace and with &&, then you have the same behavior as the if version.

3 Likes

I pattern match it apart to get h out, I didn’t do hd(list) because it was not permitted in the question. Is there another way I could do it without using hd(list)?

I’ve updated the signatures to pattern match to the boolean outputs of f.(h), does that address the above?

Ah wow, I just saw the rest of the post, fantastic!

That’s brilliant, what thought process should I use to avoid producing something clunky like mine?

implementation (with f not returning a boolean):

  all?([], _), do: true

  all?([h|t], f) do
    !f.(h)&&false||all?(t)
  end

edit:
It’s crystal clear what I need to do in future for cases like all?. Thanks for that, this is the way I want to write elixir. I’ll take a look at the second and third implementations I provided and see what I can do to make them more concise.

The main problem I’m having is that I keep losing the form that provides TCR.

This could become a problem when operating on large LTComplexity data structures?

What might be a good work around?