Re: Thinking Aloud: Refactoring from nested loops to enumerators and lambda's
John Carter <[email protected]> Mon, 1 Jul 2019 17:46:47 +1200
| Newsgroups | gmane.comp.lang.ruby.general |
|---|---|
| Message-ID | <CAFD1m3E0B7F5ky4OJaRUxKW6kDghYg6LN06ZSGSGNvoXrVfgcQ@mail.gmail.com> |
Most well written libraries tend not to have this anti-pattern...because it inhibits re-use. I designed that example to have all the worst features of that class of problem... ie. the dependencies on the parameters scattered across the whole function, and coupling between values computed in outer loops being used in the inner loops with some values factored out of the loops. > but my initial impression is to break it apart into simple-use functions that do one and only one thing, and compose those in turn. The gotcha with that is unless you're careful, the inner functions end up recomputing values on every iteration, or you pass in a largish collection of parameters which are really tightly coupled to the inner function and are entirely uncoupled to the outer functions. ie. You cannot reuse the inner functions because they are tightly coupled to the outer. On Mon, Jul 1, 2019 at 4:30 PM Brandon Weaver <[email protected]> wrote: > > Do you have a real world example of a code that needs this pattern? The problem with generalized or anonymized examples is they lack the context that would make solving the problem clearer. > > I would suggest making a minimal example that is tested that you can share with valid input and what output you expect it to return. I suspect that you're conflating a lot of ideas here but can't really understand the code from the examples. > > I'll have to reread the code a few times to understand what you're doing here, but my initial impression is to break it apart into simple-use functions that do one and only one thing, and compose those in turn. > > On Sun, Jun 30, 2019 at 11:13 PM John Carter <[email protected]> wrote: >> >> So I'm just thinking out aloud here. >> >> Being an Old School Programmer I tend to naturally to write ever more >> deeply nested loops. >> >> I hate myself when I do this because it’s hard to test especially if >> some of the loops have nasty external side effects, it’s hard to >> reuse, it’s hard to refactor. >> >> So I’m trying two new patterns.... >> >> * Passing enumerators as parameters so I can pull out inner loops as >> standalone functions without calling the entire function every time.. >> >> * Creating functions that return lambdas, so I can pass an inner loop >> in as a parameter. >> >> Walk with me this is going to be long…. the example is a teaching >> example / dojo exercise for myself, so excuse me it it slightly >> contrived. >> >> (In the following, a line of ===== indicates the next, slightly >> different version of the code) >> >> ========================================== >> >> Here is a typical chunk of my code… >> >> def nested( a, b, c) >> >> stuff_a = func_a(a) >> stuff_b = func_b(b) >> stuff_c = func_c(c) >> result = {} >> func_1( stuff_a) do |a1| >> stuff_d = func_d( a1 + stuff_b) >> func_2( stuff_d) do |a2| >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> >> result[a3] = func_f( a3) >> end >> end >> end >> >> result >> end >> >> It’s fairly clear but has a few gotchas. >> >> * It’s unclear which part of the code actually depends on which parameters. >> * In this toy example, the function is small… but a Real Life nested >> loop function like this can quickly grow hideously large. >> * As a “premature optimization” I have factored out subexpressions >> that do not alter within the loops, resulting in large scopes for >> variables that are only used inside the loops. >> * func_1(), func_2(), func_3() yield a stream of things….but a stream >> of things should just be an enumerable! >> >> ========================================== >> >> Ok, so try 2… reduce the scope of the stuff_* variables, a pessimation.. >> >> def nested( a, b, c) >> >> result = {} >> stuff_a = func_a(a) >> func_1( stuff_a) do |a1| >> stuff_b = func_b(b) >> stuff_d = func_d( a1 + stuff_b) >> func_2( stuff_d) do |a2| >> stuff_c = func_c(c) >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> >> result[a3] = func_f( a3) >> end >> end >> end >> >> result >> end >> >> >> ========================================== >> >> I can extract the inner loop as a function, but my parameter list balloons… >> >> def inner_2( a2, c, result) >> stuff_c = func_c(c) >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> result[a3] = func_f( a3) >> end >> end >> >> def nested( a, b, c) >> >> result = {} >> stuff_a = func_a(a) >> func_1( stuff_a) do |a1| >> stuff_b = func_b(b) >> stuff_d = func_d( a1 + stuff_b) >> func_2( stuff_d) do |a2| >> inner_2( a2, c, result) >> end >> end >> >> result >> end >> >> and I still reevaluate func_c for every loop! >> >> ========================================== >> If I use a closure instead, my parameter list collapses again… >> >> def nested( a, b, c) >> >> result = {} >> stuff_a = func_a(a) >> func_1( stuff_a) do |a1| >> stuff_b = func_b(b) >> stuff_d = func_d( a1 + stuff_b) >> inner_2 = ->( a2){ >> stuff_c = func_c(c) >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> result[a3] = func_f( a3) >> end >> } >> func_2( stuff_d) do |a2| >> inner_2.call( a2) >> end >> end >> >> result >> end >> >> ========================================== >> But I still have a pessimization, so if I could pass an enumerator >> around…. So lets try convert func_2 to an enumerator…. >> >> def func_2( j) >> return to_enum( __method__, j) unless block_given? >> .....lots of code and a... >> yield a2 >> ...lots more code >> end >> >> def nested( a, b, c) >> >> result = {} >> stuff_a = func_a(a) >> func_1( stuff_a) do |a1| >> stuff_b = func_b(b) >> stuff_d = func_d( a1 + stuff_b) >> inner_2 = ->( a2){ >> stuff_c = func_c(c) >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> result[a3] = func_f( a3) >> end >> } >> func_2( stuff_d).each do |a2| >> inner_2.call( a2) >> end >> end >> >> result >> end >> >> ========================================== >> >> Then pass the enumerator in, and then we can stop the silly >> re-evaluation of func_c on every loop… >> >> def nested( a, b, c) >> >> result = {} >> stuff_a = func_a(a) >> func_1( stuff_a) do |a1| >> stuff_b = func_b(b) >> stuff_d = func_d( a1 + stuff_b) >> inner_2 = ->( e){ >> stuff_c = func_c(c) >> >> e.each do |a2| >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> result[a3] = func_f( a3) >> end >> end >> } >> >> inner_2.call( func_2( stuff_d)) >> end >> >> result >> end >> >> ========================================== >> >> And I can keep going with func_1…. >> >> def nested( a, b, c) >> >> result = {} >> stuff_a = func_a(a) >> inner_1 = ->( e1, inner_2){ >> stuff_b = func_b(b) >> e1.each do |a1| >> stuff_d = func_d( a1 + stuff_b) >> >> inner_2.call( func_2( stuff_d)) >> end >> } >> >> inner_2 = ->( e2){ >> stuff_c = func_c(c) >> >> e2.each do |a2| >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e) do |a3| >> result[a3] = func_f( a3) >> end >> end >> } >> >> >> inner_1.call( func_1( stuff_a), inner_2) >> >> >> result >> end >> >> ========================================== >> >> I can reduce scope of result and move it to the outermost level…. >> >> def nested( a, b, c) >> >> stuff_a = func_a(a) >> >> inner_1 = ->( e1, inner_2, &block){ >> stuff_b = func_b(b) >> e1.each do |a1| >> stuff_d = func_d( a1 + stuff_b) >> >> inner_2.call( func_2( stuff_d), &block) >> end >> } >> >> inner_2 = ->( e2,&block){ >> stuff_c = func_c(c) >> >> e2.each do |a2| >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e,&block) >> end >> } >> >> >> result = {} >> inner_1.call( func_1( stuff_a), inner_2) do |a3| >> result[a3] = func_f( a3) >> end >> result >> end >> >> ========================================== >> >> I can convert the inner_1 lambda to a vanilla method and convert that >> to an Enumerator… >> >> def inner_1( b, e1, inner_2, &block) >> return to_enum( __method__, b, e1, inner_2) unless block_given? >> stuff_b = func_b(b) >> e1.each do |a1| >> stuff_d = func_d( a1 + stuff_b) >> >> inner_2.call( func_2( stuff_d), &block) >> end >> end >> >> def nested( a, b, c) >> >> stuff_a = func_a(a) >> >> >> inner_2 = ->( e2,&block){ >> stuff_c = func_c(c) >> >> e2.each do |a2| >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e,&block) >> end >> } >> >> >> result = {} >> inner_1( b, func_1( stuff_a), inner_2).each do |a3| >> result[a3] = func_f( a3) >> end >> result >> end >> >> ========================================== >> >> Since inner_1 is just a vanilla enum, I can use each_with_object >> >> I can also extract inner_2 as a method that returns a lambda… >> >> def inner_1( b, e1, inner_2, &block) >> return to_enum( __method__, b, e1, inner_2) unless block_given? >> stuff_b = func_b(b) >> e1.each do |a1| >> stuff_d = func_d( a1 + stuff_b) >> >> inner_2.call( func_2( stuff_d), &block) >> end >> end >> >> def inner_2( c) >> stuff_c = func_c(c) >> >> ->( e2,&block){ >> >> e2.each do |a2| >> stuff_e = func_e( a2 + stuff_c) >> >> func_3( stuff_e,&block) >> end >> } >> end >> >> def nested( a, b, c) >> >> stuff_a = func_a(a) >> >> inner_1( b, func_1( stuff_a), inner_2( c)).each_with_object({}) do >> |a3, result| >> result[a3] = func_f( a3) >> end >> end >> >> Note func_c is now only evaluated once again, It’s testable it, it’s >> reusable in the vanilla “everything is just Enumerable” sense. >> >> Comments, suggestions, nitpicks welcome.... >> >> As I said, I'm just thinking aloud trying to fix a common anti-pattern >> in my own code. >> >> -- >> John Carter >> Tait Electronics >> PO Box 1645 Christchurch >> New Zealand >> >> -- >> This Communication is Confidential. We only send and receive email on the >> >> basis of the terms set out at www.taitradio.com/email_disclaimer >> <http://www.taitradio.com/email_disclaimer> >> >> Unsubscribe: <mailto:[email protected]?subject=unsubscribe> >> <http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk> > > > Unsubscribe: <mailto:[email protected]?subject=unsubscribe> > <http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk> -- John Carter Phone : (64)(3) 358 6639 Tait Electronics PO Box 1645 Christchurch New Zealand -- This Communication is Confidential. We only send and receive email on the basis of the terms set out at www.taitradio.com/email_disclaimer <http://www.taitradio.com/email_disclaimer> Unsubscribe: <mailto:[email protected]?subject=unsubscribe> <http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk>