DEV Community

Discussion on: Help Me Refactor dev.to's Markdown Service!

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
user_name profile image
User_name

Love these smaller methods.

Easy to read && easy to follow.

Learned some neat refactoring tips-- thanks!

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

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