I originally sent this to David offline, but he suggested posting it. Some of his comments: ... Interesting that our refactorings cancelled each other out :-) My taste still runs to doing just the one loop, though I certainly see your reasoning. It would be interesting to see what other people come up with too. <End David's comments> The rest of the message follows recommended ruby-talk formatting. Sorry I do not know how to put this is a mono-spaced font, hope it transmits well. Hello -- >> On Thu, 19 Apr 2001, John Kaurin wrote: >> I made some modifications to the code supplied by >> Pete Kernan [ruby-talk 13597]. Hope this is of >> interest. >> >> # Re: [ruby-talk 13589, 13593, 13594, 13597] >> >> # Determines if an array's elements are in sequential >> # order or the order specified by an optional block. >> >> class Array >> def seq? >> return true if length <= 1 >> if block_given? >> (0..length-2).to_a.each do |i| >> return false unless yield (self[i]) == self[i + 1] >> end >> true >> else >> (0..length-2).to_a.each do |i| >> return false unless self[i].next == self[i + 1] >> end >> true >> end >> end >> end >You might want to refactor a little so that you don't have (almost) >the same code twice. Also you don't need to test for length, because >if length <= 1 the do block won't be executed at all. And you don't >have to call #to_a on the range. (Isn't that a song? :-) > class Array > def seq? > (0..length-2).each do |i| > return false unless self[i + 1] == > if block_given? > yield self[i] > else > self[i].next > end > end > true > end > end >David Thanks for your suggestions. I was able to improve my code because of them. Maybe that XP pair programming idea really works ;-) See method seq3? in the code below for my new improvements. My first cut at modifying Pete Kernan's example was code that almost exactly matched yours. Then I refactored it :-) It seemed to me that block_given? was being called needlessly for every iteration of the loop, but did not relate to the loop or its function. My gut said this was probably a performance issue for large arrays so I moved the block_given? call outside the loop, which required the appearance of duplication of code, but seemed inline with the Refactoring way of thinking. Anyway after seeing your suggestions I ran some actual testing to see if it was my gut or just gas :-) Testing reports an approximate 10% improvement when block_given? is moved outside the loop for a 500,001 element array of integers, but I am not sure if that is significant. I included test code and results below for your viewing pleasure. Thanks in any case. ---------------------Cut Here-------------------------------------- class Array def seq1? # David Black's suggestion. (0..length-2).each do |i| return false unless self[i + 1] == if block_given? yield self[i] else self[i].next end end true end def seq2? # Original post. return true if length <= 1 if block_given? (0..length-2).each do |i| return false unless yield (self[i]) == self[i + 1] end true else (0..length-2).each do |i| return false unless self[i].next == self[i + 1] end true end end def seq3? # Original post without unnecessary code. if block_given? (0..length-2).each do |i| return false unless yield (self[i]) == self[i + 1] end else (0..length-2).each do |i| return false unless self[i].next == self[i + 1] end end true end end require "benchmark" include Benchmark a1 = (0..500000).to_a bm(12) do |test| puts "seq1?:" test.report("block_given? inside loop (without block):") do a1.seq1? end puts "seq1?:" test.report("block_given? inside loop (with block):") do a1.seq1? {|x| x + 1} end puts "seq2?:" test.report("block_given? outside loop (without block):") do a1.seq2? end puts "seq2?:" test.report("block_given? outside loop (with block):") do a1.seq2? {|x| x + 1} end puts "seq3?:" test.report("block_given? outside loop (without block):") do a1.seq3? end puts "seq3?:" test.report("block_given? outside loop (with block):") do a1.seq3? {|x| x + 1} end end ---------------------Cut Here-------------------------------------- My system at work (ruby 1.6.3 (2001-03-19) [alphaev6-osf4.0f]) gives: user system total real seq1?: block_given? inside loop (without block): 3.600000 0.000000 3.600000 ( 14.125534) seq1?: block_given? inside loop (with block): 10.050000 0.033333 10.083333 ( 37.952742) seq2?: block_given? outside loop (without block): 3.166667 0.000000 3.166667 ( 11.513105) seq2?: block_given? outside loop (with block): 8.950000 0.016667 8.966667 ( 33.622730) seq3?: block_given? outside loop (without block): 3.083333 0.000000 3.083333 ( 10.295878) seq3?: block_given? outside loop (with block): 8.816667 0.016667 8.833333 ( 30.213270)