Skip to content

Conversation

@paneq
Copy link
Member

@paneq paneq commented Sep 14, 2017

that makes testing much easier when commands are used to build state.
Developers can easily opt-out by doing

def apply(*events)
  super
  nil
end

in their class.

or by explicitly returning nil or self

  def supply(quantity)
    raise NotRegistered unless @store_id
    apply(ProductSupplied.new(data: {
      store_id: @store_id,
      sku: @sku,
      quantity: quantity,
    }))
    nil
  end

that makes testing much easier when commands are used to build state.
Developers can easily opt-out by doing

def apply(*events)
  super
  nil
end

in their class.

or by explicitly returning nil or self

  def supply(quantity)
    raise NotRegistered unless @store_id
    apply(ProductSupplied.new(data: {
      store_id: @store_id,
      sku: @sku,
      quantity: quantity,
    }))
    nil
  end
@paneq
Copy link
Member Author

paneq commented Sep 14, 2017

Related to #109

@mostlyobvious
Copy link
Member

Seems like a step in good direction without causing much trouble.

@mlomnicki
Copy link
Member

I think apply should be private although that's a separate issue.

Overall this idea is an improvement to what we have currently so +1

@andrzejkrzywda
Copy link
Contributor

A good change. Should the README/release_notes change be part of the PR, given that we change the public API?

@paneq
Copy link
Member Author

paneq commented Sep 14, 2017

@andrzejkrzywda yep! Will do when we release.

@mostlyobvious mostlyobvious merged commit 0582188 into master Sep 14, 2017
@mostlyobvious mostlyobvious deleted the apply_returns_events branch September 14, 2017 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants