Andy Zhao (he/him)

Posted on Feb 1, 2018

Help Me Refactor dev.to's Markdown Service!

#meta #refactorit #ruby #help

So I'm trying to refactor one of our services that checks the Markdown before cleaning it up a bit more.

Here's the original:

# instance method in article.rb
def fix_markdown
  fixed_body_markdown = MarkdownFixer.new(body_markdown).fix_all
  fixed_body_markdown.gsub!("\r\n", "\n")
  fixed_body_markdown.gsub!(/\ntags:.*\n/) do |tags|
    tags.split(" #").join(",").gsub("#", "").gsub(":,", ": ")
  end
  fixed_body_markdown
end

# markdown_fixer.rb service

class MarkdownFixer
  attr_accessor :markdown

  def initialize(markdown)
    @markdown = markdown
  end

  # This turns --- into ------- after the first two,
  # because --- messes with front matter
  def modify_hr_tags
    markdown.gsub(/^---/).with_index { |m, i| i > 1 ? "#{m}-----" : m }
  end
end

The goal is to put the fix_markdown method from article.rb into MarkdownFixer.rb, and add an additional step to add quotation marks around the title, add_quotes_to_title. The tricky part is that I need the return value of each .gsub method call, and I personally think chaining the methods over and over would not be readable.

class MarkdownFixer
  attr_accessor :markdown

  def initialize(markdown)
    @markdown = markdown
  end

  def fix_all
    # this part is my pain point
    markdown = add_quotes_to_title
    markdown = modify_hr_tags
    markdown = markdown.gsub!("\r\n", "\n")
    markdown.gsub!(/\ntags:.*\n/) do |tags|
      tags.split(" #").join(",").gsub("#", "").gsub(":,", ": ")
    end
  end

  def add_quotes_to_title
    quoted_beginning = markdown.gsub("title: ", "title: \"")
    quoted_beginning.gsub("\r\npublished: ", "\"\r\npublished: ")
  end

  # This turns --- into ------- after the first two,
  # because --- messes with front matter
  def modify_hr_tags
    markdown.gsub(/^---/).with_index { |m, i| i > 1 ? "#{m}-----" : m }
  end
end

ANYWAY, I'm not too big of a fan of setting markdown over and over again, but I'm not sure of a better way of cleaning up the fix_all method. Would love to hear your thoughts.

Top comments (9)

rhymes • Feb 1 '18 • Edited

Hi Andy,

I thought about three variations.

The first one uses gsub! and method chaining. Each method has its own clear responsibility and the "entry point" method, fix_all just chains the calls:

class MarkdownFixer1
  attr_reader :markdown

  def initialize(markdown)
    @markdown = markdown
  end

  def fix_all
    add_quotes_to_title()
        .modify_hr_tags()
        .convert_newlines()
        .split_tags()
    markdown
  end

  def add_quotes_to_title
    @markdown.gsub!('title', '"title"')
    self
  end

  def modify_hr_tags
    @markdown.gsub!('hr', 'hhr')
    self
  end

  def convert_newlines
    @markdown.gsub!('\r\n', '\n')
    self
  end

  def split_tags
    @markdown.gsub!('tags', 't a g s')
    self
  end
end

puts MarkdownFixer1.new('title hr tags\r\n').fix_all

The second option is a variation on the first one, because looking at your example I see that the MarkdownFixer is a throw away object, it doesn't really need to hold state, just to transform it and return it parsed:

class MarkdownFixer2
  class << self
    def fix_all(markdown)
        @markdown = markdown

        add_quotes_to_title()
            .modify_hr_tags()
            .convert_newlines()
            .split_tags()
        markdown
    end

    def add_quotes_to_title
        @markdown.gsub!('title', '"title"')
        self
    end

    def modify_hr_tags
        @markdown.gsub!('hr', 'hhr')
        self
    end

    def convert_newlines
        @markdown.gsub!('\r\n', '\n')
        self
    end

    def split_tags
        @markdown.gsub!('tags', 't a g s')
        self
    end
  end
end

puts MarkdownFixer2.fix_all('title hr tags\r\n')

The third one is "almost" functional, in the sense there's no state at all and the transformations pass down the value (especially because the order is not exactly important):

class MarkdownFixer3
  class << self
    def fix_all(markdown)
        split_tags(
            convert_newlines(
                modify_hr_tags(
                    add_quotes_to_title(markdown))))
    end

    def add_quotes_to_title(markdown)
        markdown.gsub('title', '"title"')
    end

    def modify_hr_tags(markdown)
        markdown.gsub('hr', 'hhr')
    end

    def convert_newlines(markdown)
        markdown.gsub('\r\n', '\n')
    end

    def split_tags(markdown)
        markdown.gsub('tags', 't a g s')
    end
  end
end

puts MarkdownFixer3.fix_all('title hr tags\r\n')

You might also look into the visitor pattern which is definitely overkill for this example :D

rhymes • Feb 1 '18 • Edited

This is my favorite one for now :D

class MarkdownFixer4
  class << self
    def fix_all(markdown)
        methods = [:add_quotes_to_title, :modify_hr_tags, :convert_newlines, :split_tags]
        methods.inject(markdown){|result, method| self.send(method, result) }
    end

    def add_quotes_to_title(markdown)
        markdown.gsub('title', '"title"')
    end

    def modify_hr_tags(markdown)
        markdown.gsub('hr', 'hhr')
    end

    def convert_newlines(markdown)
        markdown.gsub('\r\n', '\n')
    end

    def split_tags(markdown)
        markdown.gsub('tags', 't a g s')
    end
  end
end

puts MarkdownFixer4.fix_all('title hr tags\r\n')

This line methods.inject(markdown){|result, method| self.send(method, result) } calls each method in the array passing the result of the previous method the following one, with markdown as the initial value.

Andy Zhao (he/him) • Feb 1 '18

Hmmmmm this one is pretty great! I like the use inject a lot. Thanks a lot for the answers! I'm going to think it over on my commute home. :)

User_name • Feb 3 '18

Love these smaller methods.

Easy to read && easy to follow.

Learned some neat refactoring tips-- thanks!

Seiei Miyagi • Feb 12 '18

refine the String class then just chaining methods at fix_all

class MarkdownFixer < Struct.new(:markdown)
  using Module.new {
    refine(String)do
      def add_quotes_to_title
        gsub('title', '"title"')
      end

      def add_quotes_to_published
        gsub("\r\npublished: ", "\"\r\npublished: ")
      end

      # This turns --- into ------- after the first two,
      # because --- messes with front matter
      def modify_hr_tags
        gsub(/^---/).with_index { |m, i| i > 1 ? "#{m}-----" : m }
      end

      def convert_newline
        gsub("\r\n", "\n")
      end

      def modify_tags
        gsub(/\ntags:.*\n/) do |tags|
          tags.split(" #").join(",").gsub("#", "").gsub(":,", ": ")
        end
      end
    end
  }

  def fix_all
    markdown
      .add_quotes_to_title
      .modify_hr_tags
      .convert_newline
      .add_quotes_to_title
      .add_quotes_to_published
      .moidfy_tags
  end
end

Vincenzo • Feb 2 '18

Dev.to is written in Ruby?😮🙃🤐
removes his account

Niklas • Feb 7 '18

Why do you use classes here? As i understand there is no reason to instantiate the MarkdownFixer. A module could be a more meaningful solution. What do you think? 😁

Niklas • Feb 8 '18

Another more dry approach:

Niklas • Feb 8 '18

What do you think about this: :)

DEV Community

Help Me Refactor dev.to's Markdown Service!

Top comments (9)

Read next

Ruby on Rails 8: Custom Compression for Encrypted Data

Ruby on Rails: Autenticação utilizando Devise + Keycloak

Writing Skills: Essential Fuel for Every Programmer 🥇👨‍💻

A Comprehensive Guide to Multithreading in Ruby