nix2intel
First Elixir Project (Probably terrible code)
I just finished the first iteration of my first Elixir project. I’m still super new to all of this and would love feedback, pull requests, etc. The code is available here GitHub - nix2intel/builtwith: Builtwith API wrapper written in Elixir and the documentation is here on hex builtwith. I would love to know if I’m doing dumb things, if there are different ways I should approach the problems, etc. I love elixir so far but still trying to grok it as python has been my main programming language for years.
Most Liked
nix2intel
Pattern-matchings is blowing my mind, thank you! goodbye python, this is so much cooler!
D4no0
Maybe Req is now the cool kid in town, however HttpPoison is a battle-tested library that was long before req was created, I use it a lot even to this day. Personally I would just decouple the http client and have a default implementation with whatever is easier to use these days.
D4no0
Instead of using HttpPoison directly in your codebase, you can define a generic http client interface that is fully configurable.
Here is an example of a interface: lib/ssl_moon/network/http/http_client.ex · main · SSL MOON / SSL MOON · GitLab
And implementation: lib/ssl_moon/network/http/http_client_impl.ex · main · SSL MOON / SSL MOON · GitLab
If you plan on using that code, don’t mind the logic related to redirects as that is irrelevant in your case.
seeplusplus
Some other general feedback (not Elixir specific, hope that’s ok):
Improve names
- Method names like
make_requestcan feel obvious in the moment you are writing the code, but don’t truly communicate what they are doing. Instead, considerlookup_by_domain.
Do one thing
- Most of the
get_methods are doing more than one thing, for example:
def get_subdomains(builtwithjson) do
get_results(builtwithjson)
|> Enum.find(fn map -> Map.has_key?(map, "Result") end)
|> case do
nil ->
nil
subdomains ->
Map.get(subdomains, "Result")
|> Map.get("Paths")
|> Enum.map(fn x -> Map.get(x, "SubDomain") end)
|> Enum.reject(&(&1 == ""))
end
end
You are getting the first result from a list of results and getting the SubDomain from it. I know most likely the first result of the list is probably the correct one, otherwise you wouldn’t have built it this way. The issue though is that your library is making a choice that’s hidden from the user. Namely, which one of the results is “correct.” What if my lookup by domain had multiple results and I actually wanted the second one? Instead, make these get_ methods only work with a Result, not a list of Results, e.g. change the above to:
def get_subdomains(builtwithjson_result) do
builtwithjson_result
|> Map.get(subdomains, "Result")
|> Map.get("Paths")
|> Enum.map(fn x -> Map.get(x, "SubDomain") end)
|> Enum.reject(&(&1 == ""))
end
Now, you do force the library user to pick which result they want, and they could get an error when using the get_ methods, but your current library has the following behavior:
# Imagine the list of results has, in order:
# 1. A result with a "Result" field, but no "Attributes" field.
# 2. A result with an "Atttribute" field.
results = Builtwith.lookup_by_domain("google.com", my_api_key)
# Reads the "SubDomain" field from the first entry in the list of results.
subdomains = results |> Builtwith.get_subdomains()
# Read the Attributes field from the second entry in the list of results.
attributes = results |> Builtwith.get_attributes()
The user now has a list of subdomains and a list of attributes, but they aren’t from the same result. This could be very bad for some APIs. This exact scenario may be impossible with the Builtwith API, but it could be possible with another API with a different data model and could have very bad consequences if you get this mixed up.
Micro optimization: Avoid double Enum calls
This is a small one and someone may correct me if I’m incorrect about this. In get_subdomains:
|> Enum.map(fn x -> Map.get(x, "SubDomain") end)
|> Enum.reject(&(&1 == ""))
Each call to Enum will basically allocate a new list (which will increase memory usage). Instead you can use the Stream API:
|> Stream.map(fn x -> Map.get(x, "SubDomain") end)
|> Enum.reject(&(&1 == ""))
This will lead to a slight optimization to the memory footprint of your code. Note, Streams are lazy - the output of a Stream method is always a Stream, so you will always need to end with an Enum method to get the type you actually want at the end.







