I am starting to see the design philosophy of Elixir is to inspire neurotic cleanliness with endless warnings. Which isn’t a bad thing. Keeping them all clean should help avoid error.
But what do I do with this one?
defp load_file_from_disk() do
state = %{}
mix_loaded = function_exported?(Mix, :__info__, 1)
# true with mix (debug)
# false in release (build)
path_to_file = if (mix_loaded) do
"priv"
else
:code.priv_dir(:main_server)
end
final_path = Path.join(path_to_file, "file.txt")
{_status, file_string} = File.read(final_path)
state = case (file_string) do
:enoent ->
IO.puts("ERROR: COULD NOT LOAD TOKEN FILE FROM DISK, NOT FOUND")
state
_ ->
IO.puts("READ FILE FROM DISK OKAY")
state = Map.put(state, :file, file_string) # THIS LINE GIVING WARNING
end
end
It is saying
Variable “state” is unused (there is a variable with the same name in the context, use the pin operator (^) to match on it)
I am certainly using state (in this case it is a Map and I am comparing it against another map to see if something has changed).
Are you sure that particular function is the culprit or did you leave out any context? That code looks fine to me. I tried it out myself and don’t get any warnings.
Thank you! You were totally right. I didn’t read the right place it was pointing me to correctly. I will have to be more careful about these warnings. I updated my post. Sorry to waste any of your time.
Any thoughts on the corrected one? It is correct now. Thanks again.
(I then go on to add more read files to the state which is why I declared the state as the empty map at the beginning and I am just adding to it in the if section rather than making it from scratch there.)
I am late to the party but your code has a number of problems:
Branching on file_string will never go to the :enoentcase arm because you are in fact ignoring the status by putting an underscore in front of it.
You are also not pattern-matching on the correct return values, check the fixed code below.
You are using a locally hardcoded value file.txt when passing that as a function argument would be better.
It’s not clear where does state come from but I am assuming it’s not from within your function so that one is better off extracted as a function argument as well.
I’d rework your code to this:
defmodule Stuff do
def mix_exists?(), do: function_exported?(Mix, :__info__, 1)
def file_path() do
if mix_exists?() do
"priv"
else
:code.priv_dir(:main_server)
end
end
def load_file(state, name) do
path = Path.join(file_path(), name)
case File.read(path) do
{:ok, file_contents} ->
IO.puts("File #{path} was read")
# Merge the file contents with the previous state.
Map.put(state, :file, file_contents)
{:error, :enoent} ->
IO.puts("File #{path} not found")
state
{:error, other} ->
IO.puts("Should you really ignore all other errors? File path is #{path}")
state
end
end
# more stuff...
end
And then you can use it like so:
state = %{other: :stuff, has: :happened_before}
# Other code runs in the meantime...
state = Stuff.load_file(state, "file.txt")
Actually it did work as written. I was going to the :enoent state if file didn’t load correctly (eg. bad path).
If file doesn’t load it gives file_string == :enoent which is what I checked, and I was getting those errors.
I understand everyone seems to write things in Elixir based on the full tuple pattern to catch all possibilities. Whereas I was missing the theoretical possibility of catching {:error, other}. Though I don’t think that exists in this case, I will take that approach instead then. I see the reasoning now in general. Better to write for all possibilities.
Also this is not my real code but just a simplification for clarity. It is a special purpose in this section of loading and what to do. But yes a more generalized “file load” function can be made and I appreciate your comments.
Eh, there’s a balance between code length and readability.
I’ve devised a two-liner before replying but figured it would be confusing and not very self-explanatory for an Elixir beginner.
I agree completely. The biggest problem is what happens when you return to a project (or piece of a project) 6-12 months later. I prefer simple clear code with more lines. Doing less per line allows better error finding as well.
Plus loads of annotations explaining my logic to future me who will forget why I did things. There is no shortage of disk space for text files. Better to be clearer than concise.
So this was what they were talking about in the other thread with some things working based on one path or the other and distinguishing if debug vs. build environment with is_debug = function_exported?(Mix, :__info__, 1)
In any case, it seems to work in debug (iex -S mix) with both paths for this at least (priv is being copied okay to build folder so both work) so there is no need to distinguish here at least.
Keeping them all clean should help avoid error.
