Getting input/critique on how to improve a library I wrote

Is there a better place to get a library I wrote reviewed? I’m trying to write better code, but I would like someone to give me some input on how I can improve what I’ve written.

The code is here: https://github.com/pejrich/AnonymousNameGenerator

It’s a heroku-like name generator (i.e. “hairy-monkey-123”), which I’m aware is basically as overdone and simple as “Hello, world”, but my goal was to learn to write and deploy a hex lib. The other twist to my library is that you can generate predictable/consistent names, so that instead of generating a random name each time, if you wanted to generate a name for a Post/User/etc. for your site, you could generate a name, and as long as you pass the same two integers to the lib (i.e. ID, and inserted_at as an int), it will always return the same name.

If there’s a better place to post, please let me know. Thanks in advance.

1 Like

First thing that stands out to me: use @moduledoc false at the top of Adjective and Noun unless you intend for users to be calling those modules directly.

Overall everything looks pretty reasonable. Many of the examples in your documentation would be good candidates for doctests instead. https://elixir-lang.org/getting-started/mix-otp/docs-tests-and-with.html

1 Like

I see Noun and Adjective are basically data in code. In my opinion, there are more optimal formats for data. I would probably create a priv/nouns.txt and priv/adjectives.txt with one word per line. Then I’d read the files at compile time to set a module attribute that could be used in the functions. You can use @external_resource to tell Elixir that when one of those files change, it needs to recompile your module.

1 Like

What would be the best way to do this? The only way I was able to get it to compile was by creating a macro in a separate module, and using that to define the noun/adjective functions, but that broke all my module attributes, aswell as the moduledocs that interpolated using functions/module attrs. Is there a cleaner way to do what you suggest?

Enum.at might end up being quite slow for a name generator (I assume you’d have to run it often), so I’d make a lookup table with functions. Try checking out my suggestions on https://github.com/heresydev/email_guard/pull/1, most of them would apply to your project as well, I think.

In your module you could do:

@nouns "priv/nouns.txt" 
       |> File.read!() 
       |> String.split("\n")
       |> Enum.with_index()
       |> Enum.reduce(%{}, fn {noun, index}, acc ->
         Map.put(acc, index, String.trim(noun))
       end)

I incorporated @idi527’s advice as well. Instead of Enum.at you should be able to use @nouns[3] for example.

I wish Map.new got more love. For example in @blatyo last reduce step of the pipeline, it could be:

|> Map.new(fn {noun, index} -> {index, String.trim(noun)} end)

which to my eyes makes the intent clearer (you’re creating a map) and it is shorter, which is a great combination when you can achieve it.

3 Likes

@nouns[3] or any other form of a lookup in a map is usually several times slower than a function head pattern match (at least in my benchmarks with benchee).

Based on that, I’d rewrite your example as

defmodule SomeModule do
  "priv/nouns.txt" 
  |> File.stream!() # already separates by newlines
  |> Stream.map(&String.trim/1)
  |> Stream.with_index()
  |> Enum.map(fn {noun, index} ->
    def lookup(unquote(index)), do: unquote(noun)
  end)
end

# usage
SomeModule.lookup(123) # => "<some noun>"

See https://github.com/idi-ot/email_guard/blob/3777859d620e02382b7fb076dac26026f673c595/lib/email_guard/list.ex or https://github.com/elixir-plug/mime/blob/master/lib/mime/application.ex for a more “real” example of this approach.

1 Like