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

Questions about design of some parts #12

Open
OvermindDL1 opened this issue Jan 12, 2017 · 10 comments
Open

Questions about design of some parts #12

OvermindDL1 opened this issue Jan 12, 2017 · 10 comments
Labels

Comments

@OvermindDL1
Copy link

A couple questions on design decisions or perhaps requesting new helper functions.


First, why does safe return a fun instead of branching at the point of call to be used as normal. Right now at https://github.com/expede/exceptional/blob/master/lib/exceptional/safe.ex#L86 it is doing a large branching and an apply call for every call. If instead if it (or a new name) were made a macro, then it could be fully inlined instead as a calling site, so you could do this for the examples in the README.md instead:

[1,2,3] |> safe(Enum.fetch!(1))
#=> 2

[1,2,3] |> safe(Enum.fetch!(999))
#=> %Enum.OutOfBoundsError{message: "out of bounds error"}

safe(Enum.fetch!([1,2,3], 999))
#=> %Enum.OutOfBoundsError{message: "out of bounds error"}

This could help readability and especially the usage while piping. Easily implementable as well.


Second, if_exception and such have a similar setup, they take functions, where if they were macro's then the if_exception last example could become:

ArgumentError.exception("error message")
|> if_exception do
  %{message: msg} -> msg
else
  value # if the default variable name was value, could always make it configurable
end
#=> "error message"

Also easily implementable, just stuff the do body in a case statement (maybe with a default to continue passing unmatched exceptions) and the else as normal. Could even optimize it so if the do body macro list has only one element and it is not keyed on a :-> then it could be a simple pass-through, so these would both work:

ArgumentError.exception("error message")
|> if_exception do
  %{message: msg} -> msg
else
  value
end
#=> "error message"

# Or this, if the passed in variable was named something like 'exc', could give an option to rename it too if you want:
ArgumentError.exception("error message")
|> if_exception do
  handle_error(exc)
else
  value
end
#=> "error message"

Or a configurable variable name could be used like:

ArgumentError.exception("error message")
|> if_exception myvar, do
  %{message: msg} -> msg
else
  myvar
end
#=> "error message"

# Or this, if the passed in variable was named something like 'exc', could give an option to rename it too if you want:
ArgumentError.exception("error message")
|> if_exception myvar, do
  handle_error(myvar)
else
  myvar
end
#=> "error message"

And of course the else clause could be optional if it is just a pass-through anyway.


And so forth on the other parts, just to help reduce the typing required and increasing readability by removing all sorts of things like .() from many calls.

@expede
Copy link
Owner

expede commented Feb 17, 2017

I had initially written safe to be used outside of piping for reused functions. ie: wrap it once and use it everywhere. It's a great point about the piping use case. I'll have to think a bit on the specifics of an approach. I'm working some pretty wild hours right now, so don't have a ton of mind share, but wanted to let you know that I saw this and appreciate filing the ticket 😄

@expede
Copy link
Owner

expede commented Feb 17, 2017

For the other version of if_exception, the standard lib comes with exception?/1 which handles that use case.

import Exception # NOTE: "Exception", not "Exceptional"

ArgumentError.exception("error message")
|> if exception?(myvar) do
  %{message: msg} -> msg
else
  myvar
end

☝️ Very close to the suggested version, but no extra lib required 😄

EDIT: actually, it would be more like this (👇) in a pipe, I guess. (cases are also a lot more flexible than ifs, which is a nice side-benefit of this approach). Would this cover the use case that you described?

import Exception # NOTE: "Exception", not "Exceptional"

ArgumentError.exception("error message")
|> exception?()
|> case do
  %{error: msg} -> msg
  myvar -> myvar
end

@OvermindDL1
Copy link
Author

EDIT: actually, it would be more like this (👇) in a pipe, I guess. (cases are also a lot more flexible than ifs, which is a nice side-benefit of this approach). Would this cover the use case that you described?

Heh, it would, however doesn't exception?() return a bool?

/me wishes that Exception.exception? was a macro, which it so could have been... then could use it in a matchspec like:

ArgumentError.exception("error message")
|> case do
  Exception.exception?(exc) -> :blah
  val -> :success

It is implemented pretty easy if you wanted to add in such a macro? ^.^
https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/exception.ex#L43

Also, I've been busy making a typed ML-like language in elixir Macro's as well as typing elixir itself if you are curious (very much a bit of a grown-hack, needs revamping, but eh):
ML-style: https://github.com/OvermindDL1/typed_elixir/blob/master/test/ml_elixir_test.exs
TypedElixir: https://github.com/OvermindDL1/typed_elixir/blob/master/test/typed_elixir_test.exs

Can check my latest post at https://elixirforum.com/t/typed-elixir/1388/14 for an example iex session. ^.^

@expede
Copy link
Owner

expede commented Feb 17, 2017

/me wishes that Exception.exception? was a macro, which it so could have been... then could use it in a matchspec like:

Ooooh I see what you mean now. Indeed, that would be handy! That's going in the roadmap 🎉

Also, I've been busy making a typed ML-like language in elixir Macro's as well as typing elixir itself if you are curious

Nice! I'm glad to see a few of these popping up. We all need more ML-family languages in our lives!

@OvermindDL1
Copy link
Author

Nice! I'm glad to see a few of these popping up. We all need more ML-family languages in our lives!

/me loves ML languages, especially OCaml

Typing Elixir itself is... interesting though I must admit. ^.^

@OvermindDL1
Copy link
Author

Yep, definitely still interested in if_exception handling bodies instead of functions. :-)

I'd really like something like this:

blah
|> if_exception val do
  nil
else
  val + 1
end

Or something like that, be able to name what is being passed in and have the do and else bodies actual bodies instead of piping into a function, could keep the form without the named variable as it is now, that way fully backwards compatible (although adding constant values support there would be nice, and still be backwards compatible). :-)

Right now I keep doing this, which is not quite as pretty, though it works to give a name to the value (which I'm ignoring in this specific case, but still). ^.^

      Perms.verify(conn, %Perms.Account{action: :index, id: account_id, type: k})
      |> if_exception do
        case do
          _ -> nil
        end
      else
        case do
          _ -> val
        end
      end

@OvermindDL1
Copy link
Author

Although, to be honest, you know how Exception.exception? is not a macro but what it tests is easily done via pattern matching, we need a macro one so we can put it in a matching context. ^.^

Hmm... brb...

@OvermindDL1
Copy link
Author

And I've now made that specific code into:

      Perms.verify(conn, %Perms.Account{action: :index, id: account_id, type: k})
      |> case do
        match_exception -> nil
        _ -> val
      end

It matches if it is an exception struct (you can still put match_exception = exc or whatever too), maybe writing match_exception() would make it more clear it is not a variable too, hmm... It also does not check if the __struct__ entry is an atom, but eh, I don't care about that.

I'm not fully satisfied with it, but it has definitely cleaned up a good bit of code actually, so I'll keep it. I can PR it if you want (control.ex maybe? Or a new one), or with any other changes if ideas?

@OvermindDL1
Copy link
Author

OvermindDL1 commented Mar 2, 2017

That syntax would actually make a good exception test too, another idea, I've not made this (yet) but it would be easy to make:

blah
|> exception_case do
  exception = %MyException{} = exc -> blorp(exc)
  exception = %AnotherException{} -> nil
  exception = exc-> dwoop(exc) # Any other exception
  value -> value
end

Or perhaps some other keyword than exception...

I guess that is already pretty close to case do ... end with the exception matcher, except it could enforce that __struct__ is an atom() if that really matters...

@cognivore
Copy link

Was there a follow-up on this?

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

No branches or pull requests

3 participants