Code inside matches has a different context to surrounding code.

[jasper-lyons]

In this example

class InteractionManager
    def do_useful_thing_with(thing)
    end

    def handle_error_while_verbing_thing(error)
    end

    def get_called_from_outside(context)
        VerbyThingService.new.call(context).match {
            Success(thing) { do_useful_thing_with(thing) }
            Failure(error) { handle_error_while_verbing_thing(error) }
        }
    end
end

class VerbyThingService
    include Deterministic::Prelude::Result
    alias :m :method

    def call(context)
        begin
            thing = method_that_creates_thing_or_errors(context)
            Success(thing)
        rescue MySpecificError => e
            Failure(e)
        end
    end
end

The do_useful_thing_with(thing) and handle_error_while_verbing(error) are not called and an error is raised showing that they are not in the object current in context (self).

Some questions:

• This is intended behavior:
  1. Why?
  2. What am I doing wrong here and what should I be doing (opinions).
• This is emergent behavior:
  1. Should we change it so that context of the parent is available?
  2. Should it be documented?

Let me know if you need anything from me to clarify this / help out.

my justification for this approach is that the two methods above are testable in isolation which is extremely handy.

[jasper-lyons]

I think I just found the answer:here

    def match(context=nil, &block)
      context ||= block.binding.eval('self') # the instance containing the match block
      match = binding.eval('self.class::Match.new(self, context)') # the class defining the Match

In which case perhaps there should be an example of providing a context?

[jasper-lyons]

Turns out I was wrong, here you can see that the context of the match is set to the matches parent, the enum value on which we are matching.

    class DataType
      module AnyEnum
        include Deterministic::Monad

        def match(&block)
          parent.match(self, &block) # here
        end

[kliuless]

I suspect you can fix the error by doing this:

        mgr = self
        VerbyThingService.new.call(context).match {
            Success(thing) { mgr.do_useful_thing_with(thing) }
            Failure(error) { mgr.handle_error_while_verbing_thing(error) }
        }

The match method probably needs to change the notion of self inside the block in order to do its pattern-matching magic. So you can store the original self (the InteractionManager instance) into a local variable, which can then be referenced from the blocks for each match. Unfortunately, if the methods are private, you'll need to use send.

I'm not familiar with the matcher implementation, so there could be a better way to do this.

[jasper-lyons]

Yeah, I've recently done some interesting stuff with this sort of magic in ruby.

If the match method provided a context that delegates method_missing to the binding of the block that was passed in, then everything in the block's local scope would be available as well as the stuff required for Deterministic. See here

If I get the time I may try and make this work. Thing is, atm I'm using Solid Use Case for this pattern and it works without this issue.

[kliuless]

Ah, turns you you're looking at 2 different versions of deterministic. solid_use_case uses the much older 0.6.0, which held on to context of the caller of match, then ran each match block in the context of that caller.

Works on 0.6.0 (note the slightly different matcher api):

class Foo
  def process(a)
    a+1
  end
  def run
    Success(1).match {
      success {|a| process(a) }
      failure { }
    }
  end
end

Foo.new.run  # => 2

But the equivalent code on 0.15.3 doesn't preserve the caller's context. I agree that the old behavior makes more sense. Hopefully someone who knows more can say whether it's possible to bring it back.

[pzol]

this was a design choice, not to leak the inner guts to the outside world. You can experiment with the context in which matches are done here

https://github.com/pzol/deterministic/blob/master/lib/deterministic/match.rb#L5

lines 5 and 6

Piotr

On 19 February 2016 at 23:19:57 , kliuless (notifications@github.com) wrote:

Ah, turns you you're looking at 2 different versions of deterministic. solid_use_case uses the much older 0.6.0, which did context of the caller of match, then ran each match block in the context of the caller.

Works on 0.6.0 (note the slightly different matcher api):

class Foo def process(a) a+1 end def run Success(1).match { success {|a| process(a) } failure { } } end end

Foo.new.run # => 2 But the equivalent code on 0.15.3 doesn't preserve the caller's context. I agree that the old behavior makes more sense. Hopefully someone who knows more can say whether it's possible to bring it back.

— Reply to this email directly or view it on GitHub.

[kliuless]

@pzol I should clarify. It seems to me that the latest version (v0.15.3) is leaking implementation details. Sample code:

require 'bundler/inline'
gemfile do
  gem 'deterministic', '0.15.3'
end

include Deterministic::Prelude::Result

class Foo
  def process(a)
    a+1
  end
  def run
    Success(1).match {
      # Note: "a" is accessible inside the block, even though it's not a block variable
      Success(a) { process(a) }
      Failure(a) { }
    }
  end
end

p Foo.new.run  # => expected: 2

But I get:

_scratch.rb:39:in `block (2 levels) in run': undefined method `process' for #<struct a=1> (NoMethodError)

The fact that I'm seeing the struct seems like a leak. I'm expecting to be able to call process because the run method is in the context of a Foo instance.

I did a search for PatternMatching in 0.15.3 and it's never used, so the line you're referencing doesn't have any effect. (PatternMatching is used by Either in 0.6.0 so the caller's context is being set correctly in that version.)

I have a feeling that for 0.15.3, the struct "magic" is needed to support referencing the a variable, which would otherwise be impossible if the block were run using the context of the caller (the Foo instance). The old version worked because a was passed as the block variable (see the sample for 0.6.0 in my previous comment).

[pzol]

feel free to submit a PR ;)

On 22 Feb 2016, at 18:13 , kliuless notifications@github.com wrote:

@pzol https://github.com/pzol I should clarify. It seems to me that the latest version (v0.15.3) is leaking implementation details. Sample code:

require 'bundler/inline' gemfile do gem 'deterministic', '0.15.3' end

include Deterministic::Prelude::Result

class Foo def process(a) a+1 end def run Success(1).match { Success(a) {|a| process(a) } Failure(a) { } } end end

p Foo.new.run # => expected: 2 But I get:

_scratch.rb:39:in block (2 levels) in run': undefined methodprocess' for # (NoMethodError) The fact that I'm seeing the struct seems like a leak. I'm expecting to be able to call process because the run method is in the context of a Foo instance.

I did a search for PatternMatching in 0.15.3 and it's never used, so the line you're referencing doesn't have any effect. (PatternMatching is used by Either in 0.6.0 so the caller's context is being set correctly in that version.)

— Reply to this email directly or view it on GitHub https://github.com/pzol/deterministic/issues/28#issuecomment-187273664.

[kliuless]

I believe that in order to fix this, the api has to change. Two questions (Summary)

1. Is there an advantage to the newer syntax? Success(a) { a } isn't any less verbose than Success {|a| a }. I don't think we should force the syntax to look like Haskell if it makes scoping difficult.
2. Is there a need to pass the wrapped value to the block instead of the unwrapped value? See example below with the "v" block variable.

Details In this example on 0.15.3:

# suppose "a" is 1
Success(a) { process(a) }

the goal of the "fix" is to call the block in the context of the code which calls match. But this competes with the current behavior of making the matched value a available inside the block (which is done by having a struct be the block's context).

Also, currently, if the block accepts an argument, it's the wrapped value. There's no way for the block to accept a:

Success(a) {|v| process(a) }   # "v" is Success(1), not 1

Proposal. It seems better to just have the block argument be the unwrapped value, just like in 0.6.0:

# We could also require "success" to be spelled exactly the same as the variant, i.e. capitalized, to avoid any ambiguities.
Success {|a| process(a) }   # "a" is the inner value.

We can also raise an error if the arity on the block doesn't match the member count of the enum variant, e.g. a Binary(:a, :b) would require a matcher block to have arity == 2.

[pzol]

I also find the "new" behavior annoying, honestly I do only remember there was a good reason for changing it, but it eludes my memory what it was exactly.

ad 1. I wouldn’t say so, it doesn’t make sense to try to make ruby to haskellish

ad 2. I’d go for it

On 23 Feb 2016, at 19:27 , kliuless notifications@github.com wrote:

I believe that in order to fix this, the api has to change. Two questions (Summary)

1. Is there an advantage to the newer syntax? Success(a) { a } isn't any less verbose than Success {|a| a }. I don't think we should force the syntax to look like Haskell if it makes scoping difficult.
2. Is there a need to pass the wrapped value to the block instead of the unwrapped value? See example below with the "v" block variable.

Details In this example on 0.15.3:

suppose "a" is 1

Success(a) { process(a) } the goal of the "fix" is to call the block in the context of the code which calls match. But this competes with the current behavior of making the matched value a available inside the block (which is done by having a struct be the block's context).

Also, currently, if the block accepts an argument, it's the wrapped value. There's no way for the block to accept a:

Success(a) {|v| process(a) } # "v" is Success(1), not 1 Proposal. It seems better to just have the block argument be the unwrapped value, just like in 0.6.0:

We could also require "success" to be spelled exactly the same as the variant, i.e. capitalized, to avoid any ambiguities.

Success {|a| process(a) } # "a" is the inner value. We can also raise an error if the arity on the block doesn't match the member count of the enum variant, e.g. a Binary(:a, :b) would require a matcher block to have arity == 2.

— Reply to this email directly or view it on GitHub https://github.com/pzol/deterministic/issues/28#issuecomment-187830385.

[kliuless]

@pzol I made some progress on this, and I have another API question. So far, this is working for me:

OneVar(x, where{ x == 1 }) {|a| process(a) }

But notice the duplication of the wrapped value variable: first to set up the individual match (as x), and second in the block parameter (as a).

What do you think of this simpler API:

OneVar(where{ a == 1 }) {|a| process(a) }

For implementation, the matcher can retrieve the names of the block parameters (via Proc#parameters), then use them to create the Struct context for the where guard. It might take some mental adjustment, but this seems like the best approach.

When I check the block parameters, I will only allow params of type :req and :opt. Anything else will be forbidden (e.g. :rest, :keyreq, :key, :keyrest).

Also, the params are required to have names, so certain built-in ruby methods cannot be used in a variant match block. This is ok: proc {|a,b| }.parameters # => [[:opt, :a], [:opt, :b]]

This is not (unnamed param): 1.method(:+).parameters # => [[:req]]

[kliuless]

@jasper-lyons I haven't heard back from @pzol about my proposal, i.e. specifying the wrapped values only once, in the block:

some_obj.match do
  # No longer allowed: OneVar(a, ...)
  OneVar(where{ a == 1 }) {|a| process(a) }
end

If you think this is acceptable, I'll proceed with the change.

[pzol]

hey, makes sense to me, go ahead! ;)

Thanks!

On 04 Mar 2016, at 19:42 , kliuless notifications@github.com wrote:

@jasper-lyons https://github.com/jasper-lyons I haven't heard back from @pzol https://github.com/pzol about my proposal, i.e. specifying the wrapped values only once, in the block:

some_obj.match do

No longer allowed: OneVar(a, ...)

OneVar(where{ a == 1 }) {|a| process(a) } end If you think this is acceptable, I'll proceed with the change.

— Reply to this email directly or view it on GitHub https://github.com/pzol/deterministic/issues/28#issuecomment-192402623.

[kliuless]

@pzol Thanks!

[pzol]

@kliuless could you update the HISTORY.MD with your changes pls?