Automatic flowchart generator

Question

I wanted to see graphically what my programme was doing so I wrote this automatic flowchart generator:

class Object

    def log(previous_method)
        if not DEBUGGING
            return
        end
        repr = if self.class == Enumerator then self.to_a else self end
        if [Array, String].include?(repr.class) and repr.length > 100
            repr = repr.first(20) + [". . ."] + repr.last(20)
        end
        if previous_method.downcase == 'start'
            self.tap {|x| puts """
                   #{repr}
                   """}
        else
            self.tap {|x| puts """
                     | 
                     |     #{previous_method}
                     |
                     V

                   #{repr}
                   """}
         end
     end
end

Some testcases:

class Integer
    def is_digits_pow_sum?(exp)
        self                           .log("start")
            .digits                    .log("Digits")
            .map{|x| x**exp}           .log("Each digit to the power of #{exp}")
            .sum                       .log("Sum")
            .equal?(self)              .log("Equals the start? (#{self})")
    end
end


441.is_digits_pow_sum?(3)

Generates the flowchart:

               441


                 | 
                 |     Digits
                 |
                 V

               [4, 4, 1]


                 | 
                 |     Each digit to the power of 3
                 |
                 V

               [64, 64, 1]


                 | 
                 |     Sum
                 |
                 V

               129


                 | 
                 |     Equals the start? (441)
                 |
                 V

               false

---

class String
    def alpha_value
        self                           .log("Start")
            .chars                     .log("Split char by char")
            .map(&:alphabet_position)  .log("Map by position in the alphabet")
            .sum                       .log("Sum")
    end
end

"hellopeople".alpha_value

Gives the flowchart:

               hellopeople


                 | 
                 |     Split char by char
                 |
                 V

               ["h", "e", "l", "l", "o", "p", "e", "o", "p", "l", "e"]


                 | 
                 |     Map by position in the alphabet
                 |
                 V

               [8, 5, 12, 12, 15, 16, 5, 15, 16, 12, 5]


                 | 
                 |     Sum
                 |
                 V

               121

Answer 1

You're a Python native, aren't you? I can tell from the """ multiline strings and the four-space indents and really unidiomatic whitespacing and if/then/else/end instead of ternary and if not instead of unless and use of blocks instead of statements and wow this is really unidiomatic ;-;

Each tip assumes that you've already applied the last.

Also, I'm writing this review from the perspective of Ruby 2.2.2 -- some methods that existed when you wrote this don't exist any more, and some methods that do what you want better have been created. I suggest looking through the Ruby docs for the updated methods, since there's no way I caught everything.

1. Ruby indents are two spaces, not four.

2. You may have noticed a bug -- even though you seem to be specifying a single multiline string, it's printing out a bunch of newlines between the beginning and the end. That's because """ [stuff] """ is interpreted not as a single string but as three: "", " [stuff] ", and "". Then, it outputs each of those on a new line, which results in a bunch of extra spacing. It also doesn't eliminate the indentation, so it ends up being moved really far to the right, too.

   To fix both issues, you can use a heredoc:

   else
     self.tap {|x| puts <<-END.gsub(/^\s*\||#@.*$/, '')
                     |
                     |    |
                     |    |    #{previous_method}
                     |    |
                     |    V
                     |
                     | #{repr}
                   END}
   end
   

3. In multiline blocks, use do/end instead of {/}:

   else
     self.tap do |x|
       puts <<-END.gsub(/^\s*\||#@.*$/, '')
         |
         |    |
         |    |    #{previous_method}
         |    |
         |    V
         |
         | #{repr}
       END
     end
   end
   

4. Actually, why are you using self.tap at all? Just do the puts, then return self after:

   else
     puts <<-END.gsub(/^\s*\||#@.*$/, '')
       |
       |    |
       |    |    #{previous_method}
       |    |
       |    V
       |
       | #{repr}
     END
   end
   self
   

5. In Ruby, we have the ternary operator (condition ? a : b). Use it instead of the one-line if/then/else/end construction:

   repr = (self.class == Enumerator) ? self.to_a : self
   

6. But even that's unnecessary -- firstly, to_a returns an Array, not an Enumerable. Secondly, calling to_a on an Array will return the unmodified array, so you can just write:

   repr = self.to_a
   

   Of course, this is a bad idea. What happens when someone tries to use your .log with an IO? Should the entire contents of the IO be put into an array? That seems really stupid to me. Personally, I would instead have

   repr = self.to_s
   

   Since, after all, you'll be printing it, and it's far better to let the designers of an object decide how it ought to be printed, rather than trying to wrest control from the developer using the library.

7. You know, repr is really a crappy name. What does it mean? You can feel free to have long variable names in Ruby; just make sure they're in snake_case.

8. What is this?

   if [Array, String].include?(repr.class) and repr.length > 100
     repr = repr.first(20) + [". . ."] + repr.last(20)
   end
   

   You already forced repr to be a String (and in your code, it was already an Array). Why are you checking that again? Plus, if it's a String, then it'll fail when you try to call the nonexistent first on it! Besides, Array and String aren't the only classes with a .length method. This is how I'd do it (keeping in mind that in #6 I converted it to a String):

   repr = repr.length > 100 ? "#{repr[0..20]}. . .#{repr[-20..-1]}" : repr
   

   For more information about String#[], see the docs. I'd also recommend pulling this out into a method -- the method name is up to you, but I'd personally call it shorten or trim_extra_chars or something like that.

9. previous_method really isn't a very good name. It's not necessarily method that came before, after all -- it's an operation that led to the current state. It could well have been something that isn't a method! I'd call it previous_operation.

10. What happens when someone wants to log a method called start? For example, say they have an object representing a task that's built over several chained method calls, then started with start, which returns the result of the task. They'll try to .log('start'), but it'll instead give them this weird thing that they didn't expect! I'd recommend splitting it into two methods -- then, instead of an if/else and a default value, you can have:

    def log_start
        #etc.
    end
    

    and

    def log(previous_operation)
        #etc.
    end
    

    One bonus of this (combined with #8) is that you can totally get rid of repr.

11. Now, there are a couple of other things: Very rarely do people want to log stuff to the console (i.e. where puts goes). I'd recommend adding support for outputting to other streams, though there are a couple of ways to do it. One of them, my personal favorite, involves a lot of metaprogramming and probably isn't worth it. Another is just to add a parameter output=$stdin and replace all calls to puts with output.puts to log_start and log.

12. Lastly, I did some changes in the actual output, just because it looks nicer IMO. That's entirely up to you.

Now, here's your code with all of those changes applied:

class String
  def shorten
    self.length > 100 ? "#{self[0..20]}. . .#{self[-20..-1]}" : self
  end
end

class Object
  def log_start
    puts <<-END.gsub(/^\s*\||#@.*$/, '') 
      |  #{self.to_s.shorten}
    END
    self
  end

  def log(previous_method)
      puts <<-END.gsub(/^\s*\||#@.*$/, '')
        |   |
        |   |    #{previous_method}
        |   V
        |  #{self.to_s.shorten}
      END
    self
  end
end

Note: I didn't apply tip #11 because there are a bunch of ways to do it and I felt it would be better if I left it up to you how you want to do that.

To test it (Adapted to Ruby 2.2.2 but works the same):

class Integer
  def is_digits_pow_sum?(exp)
    self                           .log_start
        .to_s.chars.map(&:to_i)    .log("Get the digits")
        .map{|x| x**exp}           .log("Each digit to the power of #{exp}")
        .inject(&:+)               .log("Sum the digits")
        .equal?(self)              .log("Equals the start? (#{self})")
  end
end

441.is_digits_pow_sum?(3)

which prints

  441
   |
   |    Get the digits
   V
  [4, 4, 1]
   |
   |    Each digit to the power of 3
   V
  [64, 64, 1]
   |
   |    Sum the digits
   V
  129
   |
   |    Equals the start? (441)
   V
  false