Skip to content

Commit

Permalink
fix bug triggered by nesting ifs and eaches
Browse files Browse the repository at this point in the history
  • Loading branch information
markburns committed Dec 30, 2023
1 parent 55ac5e0 commit a78ac9c
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 18 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@
- All internal restructuring/refactoring into domains.
- Add support for organize `self.if(:condition, then: A, else: B)` syntax
- change location of matchers to `require 'interactify/rspec_matchers/matchers'`

## [0.4.1] - 2023-12-29
- Fix bug triggered when nesting each and if
2 changes: 1 addition & 1 deletion lib/interactify/dsl/each_chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def klass
end # end

define_singleton_method(:source_location) do # def self.source_location
const_source_location this.evaluating_receiver.to_s # [file, line]
const_source_location this.evaluating_receiver.to_s # [file, line]
end # end

define_method(:run!) do # def run!
Expand Down
94 changes: 87 additions & 7 deletions spec/fixtures/integration_app/app/interactors/all_the_things.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,37 @@
# This is test code.
#
# Avoid using organizers predominantly composed of lambdas. In complex organizers,
# it's common to have many isolated interactor classes, but maintain some orchestration
# code for readability and cohesion.
#
# For instance, rather than creating an EachProduct interactor or a separate interactor chain
# for a single boolean flag, it's more efficient to manage the high-level business process
# in one location. This approach reduces the need to switch between two mindsets:
#
# Mode 1 (High-Level Overview): Understanding the business process and identifying where changes fit.
# Mode 2 (Detailed Implementation): Focusing on the specific logic of a process step.
#
# Initially, in Mode 1, a bird's-eye view is essential for grasping the process architecture.
# Once this overview is clear, you pinpoint the relevant interactor for modifications or additions,
# transitioning to Mode 2 for implementation.
#
# This distinction between modes is crucial. Mode 1 involves more reading, conceptualizing,
# and questioning, often requiring navigation through multiple files and note-taking.
# Mode 2 is about active coding and editing. Minimizing context switching between these modes
# saves time and mental energy. It's also easier to maintain a high-level overview of the process
# when the code is in one place.
#
# Typically this will allow us to move closer towards a one organizer per interaction model.
# E.g. one Rails action == one organizer.
# Even the background job firing can be handled by the organizer via YourClass::Async.
#
# An improvement we aim for is automatic diagram generation from the organizers, utilizing
# contracts for documentation.
#
# Note: The conditional structures (`if`, `each`) here are not executed at runtime in the usual
# sense. They are evaluated when defining the organizer, leading to the creation of discrete
# classes that handle the respective logic.
#
class AllTheThings
include Interactify

Expand All @@ -12,34 +46,44 @@ class AllTheThings
else: [If::C, If::D]
),

self.each(
:things,
If::A,
If::B,
-> (c) { c.lambda_set = true }
# test each with lambda
self.if(
-> (c) { c.things },
self.each(
:things,
If::A,
If::B,
-> (c) { c.lambda_set = true }
),
),

# test alternative if syntax
self.if(
-> (c) { c.a && c.b } ,
then: -> (c) { c.both_a_and_b = true },
else: -> (c) { c.both_a_and_b = false}
),

# test setting a value to use later in the chain
-> (c) { c.more_things = [1, 2, 3, 4] },

# test lambdas inside each
self.each(
:more_things,
-> (c) {
if c.more_thing_index.zero?
c.first_more_thing = true
end
},
-> (c) {
-> (c) {
if c.more_thing_index == 1
c.next_more_thing = true
end
},
# test nested if inside each
{if: :not_set_thing, then: -> (c) { c.not_set_thing = true } },

# test setting a value after an else
-> (c) { c.more_thing = true }
),

Expand All @@ -49,11 +93,47 @@ class AllTheThings
-> (c) { c.optional_thing_was_set = true },
-> (c) { c.and_then_another_thing = true },
-> (c) { c.and_one_more_thing = true },
# test nested each inside if
self.each(:more_things,
-> (c) { c.more_things[c.more_thing_index] = c.more_thing + 5 },
-> (c) { c.more_things[c.more_thing_index] = c.more_thing + 5 }
)
)
],
else: -> (c) { c.optional_thing_was_set = false }
),

# if -> each -> if -> each
self.if(:condition, then: [-> {}], else: [
self.each(
:things,
self.if(
:thing,
then: self.each(
:more_things,
-> (c) {
c.counter ||= 0
c.counter += 1
}
)
)
)
]),

# each -> each -> each
self.each(
:more_things,
self.each(
:more_things,
self.each(
:more_things,
self.each(
:more_things,
-> (c) {
c.heavily_nested_counter ||= 0
c.heavily_nested_counter += 1
}
)
)
)
)
end
18 changes: 8 additions & 10 deletions spec/integration/all_the_things_integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,27 @@
expect(result.b).to eq("b")
expect(result.c).to eq(nil)
expect(result.d).to eq(nil)

expect(result.lambda_set).to eq(true)
expect(result.both_a_and_b).to eq(true)
expect(result.more_things).to eq([1, 2, 3, 4])
expect(result.first_more_thing).to eq(true)
expect(result.next_more_thing).to eq(true)
expect(result.optional_thing_was_set).to eq(false)

expect(result.counter).to eq 8
expect(result.heavily_nested_counter).to eq 256
end
end

context "with an optional thing" do
let(:result) { AllTheThings.promising(:a).call!(things:, optional_thing: true) }

it "sets A and B, then lambda_set, then both_a_and_b, then first_more_thing, next_more_thing" do
expect(result.a).to eq("a")
expect(result.b).to eq("b")
expect(result.c).to eq(nil)
expect(result.d).to eq(nil)
expect(result.lambda_set).to eq(true)
expect(result.both_a_and_b).to eq(true)
expect(result.more_things).to eq([6, 7, 8, 9])
expect(result.first_more_thing).to eq(true)
expect(result.next_more_thing).to eq(true)
it "sets the optional thing" do
expect(result.optional_thing_was_set).to eq(true)

expect(result.counter).to eq 8
expect(result.heavily_nested_counter).to eq 256
end
end

Expand Down

0 comments on commit a78ac9c

Please sign in to comment.