Skip to content
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

Please add String.to_atom(:atom) #2915

Closed
mgwidmann opened this issue Dec 3, 2014 · 6 comments
Closed

Please add String.to_atom(:atom) #2915

mgwidmann opened this issue Dec 3, 2014 · 6 comments

Comments

@mgwidmann
Copy link
Contributor

Currently to_string supports both atoms and binaries, however converting to atoms does not.

This makes it tedious to have to run values through code like value |> to_string |> String.to_atom when all that is really desired is for String.to_atom(value) to work regardless if value is a binary or an atom.

I attempted to make this modification, but it seems its far more complicated than just adding

def to_atom(atom) when is_atom(atom), do: atom

above the String.to_atom/1 definition. I found the elixir_rewrite.erl module and attempted to modify it but couldn't find what the correct solution would be to make the tests pass (otherwise this would be a pull request 😭)

@bitwalker
Copy link
Contributor

@mgwidmann I'm not sure I understand, why would you call String.to_atom with an atom?

@mgwidmann
Copy link
Contributor Author

@bitwalker Like this (only assume the data is coming from somewhere else, not hard coded):

defmodule MyModule do
  ["one_#{:something_interpolated}", :two, "three_#{:something_else}", :four] |> Enum.each fn(v)->
    def unquote(String.to_atom(v))(), do: :the_return_value
  end
end

My point is that to_string uses protocols to be able to call it on anything and it just does what you expect, to_atom does not.

@mgwidmann
Copy link
Contributor Author

I guess maybe what I'm suggesting instead is that to_atom use a protocol like String.Chars is for to_string

@josevalim
Copy link
Member

Some application can tackle to_atom or define the Atom.Chars protocol. The trouble though is that we want developers to be much more careful about converting stuff to atom as they are not garbage collected so I don't think we should ship it directly in Elixir. Thank you!

@mgwidmann
Copy link
Contributor Author

Ok, that sounds reasonable. Question though, in the above code snippet without the String.to_atom call the code fails on strings because of a syntax error def "one_something_interpolated"(), do: :the_return_value, is this a bug or should it handle conversion to atom for the method name for you?

@josevalim
Copy link
Member

It is definitely not valid syntax!

On Thursday, December 4, 2014, Matt notifications@github.com wrote:

Ok, that sounds reasonable. Question though, in the above code snippet
without the String.to_atom call the code fails on strings because of a
syntax error def "one_something_interpolated"(), do: :the_return_value,
is this a bug or should it handle conversion to atom for the method name
for you?


Reply to this email directly or view it on GitHub
#2915 (comment).

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Lead Developer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants