-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove download_urls/3 #67
Conversation
Enumerable traversal is a trivial concept. It does not make sense to have a separate function just to do Enum.map. It brings confusing function dichotomy and a need for guard clauses. Also it discourages from using other types of enumerables like streams. For the function to be testable, the get_env call needed to be moved to main/0, which makes nice symmetry with http_client. Because the input_path parameter is of the same type and thus indistiguishable from a URL, it only makes sense to rename the function.
Extracted the |
4bba653
to
1db8469
Compare
Reverted the get_env move to minimize the amount of changes. Used Enum.map in the malfunctioning “download URLs from the input file” test to at least make it still download all the URLs.
1db8469
to
8895fef
Compare
Reverted the |
Merged current elixir and resolved conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May become better after #70 is resolved. |
As mentioned in #66 (comment), enumerable traversal is a trivial concept. It does not make sense to have a separate function just to do
Enum.map
. It brings confusing function dichotomy and a need for guard clauses. Also it discourages from using other types of enumerables like streams.For the function to be testable, the
get_env
call needed to be moved tomain/0
, which makes nice symmetry withhttp_client
. Because theinput_path
parameter is of the same type and thus indistinguishable from a URL, it only makes sense to rename the function.