DEV Community

Andy Zhao (he/him)
Andy Zhao (he/him)

Posted on

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
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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)

Collapse
 
rhymes profile image
rhymes • • 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

Collapse
 
rhymes profile image
rhymes • • 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.

Collapse
 
andy profile image
Andy Zhao (he/him) •

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. :)

Collapse
 
user_name profile image
User_name •

Love these smaller methods.

Easy to read && easy to follow.

Learned some neat refactoring tips-- thanks!

Collapse
 
hanachin profile image
Seiei Miyagi •

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
Collapse
 
vikkio88 profile image
Vincenzo •

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

Collapse
 
nhh profile image
Niklas •

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? 😁

Collapse
 
nhh profile image
Niklas •

Another more dry approach:

Collapse
 
nhh profile image
Niklas •

What do you think about this: :)

Read next

alexandrecalaca profile image

How to temporarily skip the pending migration check in Rails

Alexandre Calaça -

abstractmusa profile image

Install AWS CLI (Command Line Interface) on Ubuntu

Md Abu Musa -

busycaesar profile image

How to integrate pgvector's Docker image with Langchain?

Ďēv Šhãh 🥑 -

davidjonson profile image

Best Programming Language to Develop E-commerce Platforms

David Jonson -