loading...
Cover image for Attempting to Learn Go - Building Dev Log Part 04

Attempting to Learn Go - Building Dev Log Part 04

shindakun profile image Steve Layton Updated on ・5 min read

Attempting to Learn Go (23 Part Series)

1) Attempting to Learn Go - Building a Downloader Part 01 2) Attempting to Learn Go - Building a Downloader Part 02 3 ... 21 3) Attempting to Learn Go - Building a Downloader Part 03 4) Attempting to Learn Go - Building a Downloader Part 04 5) Attempting to Learn Go - Building a Downloader Part 05 6) Attempting to Learn Go - Consuming a REST API 7) Attempting to Learn Go - Continuing REST Adventures 8) Attempting to Learn Go - Now Sending REST Requests 9) Attempting to Learn Go - REST API and A Bit On Templates 10) Attempting to Learn Go - Sending Email Via API Again 11) Attempting to Learn Go - Let's Get Modular! 12) Attempting to Learn Go - Let's Get Modular - Again! 13) Attempting to Learn Go - Building Dev Log Part 01 14) Attempting to Learn Go - Building Dev Log Part 02 15) Attempting to Learn Go - Building Dev Log Part 03 16) Attempting to Learn Go - Building Dev Log Part 04 17) Attempting to Learn Go - Building Dev Log Part 05 18) Attempting to Learn Go - Listing Files By Extension 19) Attempting to Learn Go - Sorting and Moving Files by Extension 20) Attempting to Learn Go - Issuer 01 21) Attempting to Learn Go - Issuer 02 22) Attempting to Learn Go - Issuer 03 - Cloud Function Go! 23) Attempting to Learn Go - Issuer 04 - Addendum

Building Out A Dev Blog

Look at that! We're back in action. First thing I want to start off and say thanks for the constructive comments I received on the last post. I try to take time to consider any comment - after all, I'm still learning so I (admittedly) am not doing everything correct. All I can say for sure is that the code compiled on my machine. 😜


Taking a Step Back

After looking over the code, and the comments I realized I may have been being a bit too terse with my variable names. Go is typically know for have short variable names but, it seems like it's could potentially make code harder to follow. With that in mind, I've begun to make the variable names more explicit. There are still cases where I am using a short name, b for a []byte for example. Hopefully, those cases will still be pretty clear and easy to follow since I tried to make sure they are declared near where they are used.

With that said let's take a look where we are.

package main

import (
  "bufio"
  "bytes"
  "fmt"
  "html/template"
  "io"
  "io/ioutil"
  "os"
  "regexp"
  "strings"

  "github.com/microcosm-cc/bluemonday"
  "github.com/russross/blackfriday"
  yaml "gopkg.in/yaml.v2"
)

const delimiter = "---"

type post struct {
  Title       string
  Published   bool
  Description string
  Tags        []string
  CoverImage  string
  Series      string
  PostBody    template.HTML
}

type index struct {
  Pages []Page
}

type Page struct {
  FileName string
  Title    string
}

I've added the index template and so we can generate a simple index page for the site. We pass in our index struct and range through the Pages to get each Page which contains the filename and the page title.

var indexTempl = `<!DOCTYPE html>
<html lang="en">
  <head>
    <title>shindakun's dev site</title>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="HandheldFriendly" content="True">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta name="referrer" content="no-referrer-when-downgrade" />
    <meta name="description" content="shindakun's dev site" />
  </head>
  <body>
    <div class="index">
    {{ range $key, $value := .Pages }}
      <a href="/{{ $value.FileName }}">{{ $value.Title }}</a>
    {{ end }}
    </div>
  </body>
  </html>
`

var postTempl = `<!DOCTYPE html>
<html lang="en">
  <head>
    <title>{{.Title}}</title>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="HandheldFriendly" content="True">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta name="referrer" content="no-referrer-when-downgrade" />
    <meta name="description" content="{{.Description}}" />
  </head>
  <body>
    <div class="post">
      <h1>{{.Title}}</h1>
      {{.PostBody}}
    </div>
  </body>
  </html>
`

It was pointed out by @ladydascalie in the comments that we would be better over accepting io.Reader this will allow me to load the file and easily mock tests of this function. We've also renamed the function to be much more descriptive. I decided to go with getContentsOf() so in the code it reads as getContentsOf(openedFile).

func getContentsOf(r io.Reader) ([]byte, error) {
  b, err := ioutil.ReadAll(r)
  if err != nil {
    return nil, err
  }
  return b, nil
}

func parseFrontMatter(b []byte) (map[string]interface{}, error) {
  fm := make(map[string]interface{})
  err := yaml.Unmarshal(b, &fm)
  if err != nil {
    msg := fmt.Sprintf("error: %v\ninput:\n%s", err, b)
    return nil, fmt.Errorf(msg)
  }
  return fm, nil
}

func splitData(fm []byte) ([][]byte, error) {
  b := bytes.Split(fm, []byte(delimiter))
  if len(b) < 3 || len(b[0]) != 0 {
    return nil, fmt.Errorf("Front matter is damaged")
  }
  return b, nil
}

I also took @krusenas 's suggestion to actually put some handling in place for the makePost(). We aren't always going to be working with known-good files and we really should consider that we'll have "bad" YAML before long. There is probably a less verbose way to handle this but it works for now. However, this is not done! We will have at least one more post in this part of the series, we'll need to reject the file if there is no title. After all, if there is no title we have nothing to link to in the index.

// makePost creates the post struct, returns that and the template HTML
func makePost(fm map[string]interface{}, contents []byte, s [][]byte) (*template.Template, *post) {
  post := &post{}

  // TODO: Reject post if we don't have a title
  titleIntf, ok := fm["title"]
  if ok {
    title, ok := titleIntf.(string)
    if ok {
      post.Title = title
    } else {
      post.Title = ""
    }
  } else {
    post.Title = ""
  }

  pubIntf, ok := fm["published"]
  if ok {
    published, ok := pubIntf.(bool)
    if ok {
      post.Published = published
    } else {
      post.Published = false
    }
  } else {
    post.Published = false
  }

  descIntf, ok := fm["description"]
  if ok {
    description, ok := descIntf.(string)
    if ok {
      post.Description = description
    } else {
      post.Description = ""
    }
  } else {
    post.Description = ""
  }

  tagsIntf, ok := fm["tags"]
  if ok {
    tags, ok := tagsIntf.(string)
    if ok {
      post.Tags = strings.Split(tags, ", ")
    } else {
      post.Tags = []string{}
    }
  } else {
    post.Tags = []string{}
  }

  covIntf, ok := fm["cover_image"]
  if ok {
    coverImage, ok := covIntf.(string)
    if ok {
      post.CoverImage = coverImage
    } else {
      post.CoverImage = ""
    }
  } else {
    post.CoverImage = ""
  }

  seriesIntf, ok := fm["series"]
  if ok {
    series, ok := seriesIntf.(string)
    if ok {
      post.Series = series
    } else {
      post.Series = ""
    }
  } else {
    post.Series = ""
  }

  pBody := contents[len(s[1])+(len(delimiter)*2):]

  out := blackfriday.Run(pBody)

  bm := bluemonday.UGCPolicy()
  bm.AllowAttrs("class").Matching(regexp.MustCompile("^language-[a-zA-Z0-9]+$")).OnElements("code")
  post.PostBody = template.HTML(bm.SanitizeBytes(out))

  tm := template.Must(template.New("post").Parse(postTempl))
  return tm, post
}

Our writeIndex() function is pretty simple and we might extend this out a bit so we have just one write to file function. We have basically the same code in our main loop. For now, though a little extra code is OK by me.

func writeIndex(idx index) {
  indexFile, err := os.Create("index.html")
  if err != nil {
    panic(err)
  }
  defer indexFile.Close()
  buffer := bufio.NewWriter(indexFile)
  tm := template.Must(template.New("index").Parse(indexTempl))
  err = tm.Execute(buffer, idx)
  if err != nil {
    panic(err)
  }
  buffer.Flush()
}

func main() {
  var idx index

  dir, err := ioutil.ReadDir(".")
  if err != nil {
    panic(err)
  }

  for _, file := range dir {
    if fileName := file.Name(); strings.HasSuffix(fileName, ".md") {

      openedFile, err := os.Open(fileName)
      if err != nil {
        panic(err)
      }

      contents, err := getContentsOf(openedFile)
      if err != nil {
        panic(err)
      }
      s, err := splitData(contents)
      if err != nil {
        panic(err)
      }

      fm, err := parseFrontMatter(s[1])
      if err != nil {
        msg := fmt.Sprintf("error: %v\ninput:\n%s", err, s[1])
        panic(msg)
      }

      template, post := makePost(fm, contents, s)

      trimmedName := strings.TrimSuffix(fileName, ".md")
      outputFile, err := os.Create(trimmedName + ".html")
      if err != nil {
        panic(err)
      }
      defer outputFile.Close()

      buffer := bufio.NewWriter(outputFile)

      err = template.Execute(buffer, post)
      if err != nil {
        panic(err)
      }
      buffer.Flush()

      indexLinks := Page{
        FileName: trimmedName + ".html",
        Title:    post.Title,
      }
      idx.Pages = append(idx.Pages, indexLinks)
    }
  }
  writeIndex(idx)
}

In the end, we didn't have too many changes this week just a bit of cleaning up and adding the code to write our index file. This makes out code pretty complete, not perfect since we're not able to specify input and output directories but still useable. I ran a bit short in time this week anyway so it works out, I'll try to make add our last bits to close out this part of the series over the next week. Wonder what we should tackle after that... Have any ideas?


You can find the code for this and most of the other Attempting to Learn Go posts in the repo on GitHub.



Attempting to Learn Go (23 Part Series)

1) Attempting to Learn Go - Building a Downloader Part 01 2) Attempting to Learn Go - Building a Downloader Part 02 3 ... 21 3) Attempting to Learn Go - Building a Downloader Part 03 4) Attempting to Learn Go - Building a Downloader Part 04 5) Attempting to Learn Go - Building a Downloader Part 05 6) Attempting to Learn Go - Consuming a REST API 7) Attempting to Learn Go - Continuing REST Adventures 8) Attempting to Learn Go - Now Sending REST Requests 9) Attempting to Learn Go - REST API and A Bit On Templates 10) Attempting to Learn Go - Sending Email Via API Again 11) Attempting to Learn Go - Let's Get Modular! 12) Attempting to Learn Go - Let's Get Modular - Again! 13) Attempting to Learn Go - Building Dev Log Part 01 14) Attempting to Learn Go - Building Dev Log Part 02 15) Attempting to Learn Go - Building Dev Log Part 03 16) Attempting to Learn Go - Building Dev Log Part 04 17) Attempting to Learn Go - Building Dev Log Part 05 18) Attempting to Learn Go - Listing Files By Extension 19) Attempting to Learn Go - Sorting and Moving Files by Extension 20) Attempting to Learn Go - Issuer 01 21) Attempting to Learn Go - Issuer 02 22) Attempting to Learn Go - Issuer 03 - Cloud Function Go! 23) Attempting to Learn Go - Issuer 04 - Addendum

Posted on by:

shindakun profile

Steve Layton

@shindakun

I've been known to write some code from time to time.

Discussion

markdown guide
 

Thanks for including me in your article!
I'm going to jump right back in and point out the following:

b, err := ioutil.ReadAll(r)
if err != nil {
    return nil, err
}
return b, nil

Can be simplified to:

return ioutil.ReadAll(r)

Which means, incidentally, you can get rid of getContentsOf altogether if you wanted to!

I'll also point out that you might want clearer errors in some places.
Now this is slightly nit-picky, given the context of what you are doing, but consider this:

contents, err := getContentsOf(openedFile)
if err != nil {
    fmt.Errorf("failed getting contents of file %s: %v", openedFile.Name, err)
}

You also have a couple cases of very nested / complex if/else blocks.

I'll pick on this one:

 descIntf, ok := fm["description"]
  if ok {
    description, ok := descIntf.(string)
    if ok {
      post.Description = description
    } else {
      post.Description = ""
    }
  } else {
    post.Description = ""
  }

You could refactor it in the following way:

descIntf, ok := fm["description"]
post.Description = ""
if ok {
    if desc, ok := descIntf.(string); ok {
        post.Description = desc
    }
}

The interesting part in this one is that you make the default case useful (set the description to empty string), and only act when your conditions are met.

Classic case where flipping the logic can make your code cleaner!

 

Thanks so much for the comments! Part of the reason for these posts is to not to just learn Go but put stuff out there and find out if there are "proper" (or alternate) way to do things. I was worried at first about posting since I'm kind of just going for it, the code works but definitely wouldn't be "production" quality. My hope was, (and it looks like it's being realized) that people such as yourself would take the time to comment and help where things come up.

I do need to rethink the way I'm writing these though as I typically just leave panic()'s all over the place which is fine for these kinds of projects but, really not good form. Moving to more detailed error messages and properly dealing with them would make everything more robust. I think a change in habit is really what is needed.

The nested ifs really left a bad smell, I almost just left all the checking out again but in the end, I decided some checking is better than none. There is a post from Mat Ryer which I kept meaning to go back to just for this section of code. Which I think is right in line with your comments. Your refactor is much easier to follow the the orignial and would drastically improve the overall readability of the makePost function.

Thanks again! 😃

 

definitely wouldn't be "production" quality

If it works, it works! As long as you don't have egregious design flaws, you can always refactor to a clean codebase.

I think a change in habit is really what is needed

That is quite accurate. I remember the early days of using Go within my team. people just starting to use the language would frequently make that same mistake of either just retuning all errors up the call stack, or using panic when they didn't really know how to respond to the failure.

In your case, it very well may be that you do not want to continue processing any further in case of an error, and there's nothing inherently wrong with that. Having detailed messages will help you track down your issues easily.

Here's an interesting tidbit for you to consider as well:

If you've learned about panic and recover, then you'll know that one of the interesting properties of panic is that it will still run defered function calls. If you haven't read up on that, I encourage you to read this article: golangbot.com/panic-and-recover

In your case, that means that if you open a file, and then run defer f.Close(), even if you were to panic later on, your will try and cleanup the file by closing it properly.

However, a naked panic call only takes in a string as a message, and so it is unwieldy to construct proper error messages with it.

The solution here would be to use log.Panicf, which the docs describe as "[...]equivalent to Printf() followed by a call to panic()."

That makes it easier to use, and you can reuse known idioms:

log.Panicf("releveant error context: %v", err).

Of course you will have scenarios where you do not want to panic immediately, and you do want to return the error up the callstack.

In this case, I generally do this, if I do not control the source of the error, or don't care about it very much.

thing, err := something() {
    return fmt.Errorf("wrapped error: %v", err)
}

If I do control it, or care, then I will approach this two ways:

You can easily create your own error types, to make control flow easier on yourself:

type ParseError struct { error }

Simply embedding error into the struct makes this a one liner:

fm, err := parseFrontMatter(input)
if err != nil {
    return ParseError{error:fmt.Errorf("could not parse front matter with error: %v", err)}
}

Now, instantly, you get more context, and you are also able to do this, higher up the call stack:

if err := process(thing); err != nil {
    switch e := err.(type) {
        case ParseError:
             // Do what you need in that case
        default:
             log.Panicf("Unexpected error %T: %[1]v", e)
    }
}

Side note:

This line log.Panicf("Unexpected error %T: %[1]v", e) uses explicit argument indexes, which are documented in the fmt package.

I wish I could like this twice. ;) Thanks so much for the detailed comment and the links! panic and recover are on my list of things to get to, I've only given recover a cursory glance so far. And honestly, I didn't realize log.Panicf() was a thing, don't recall seeing it used anywhere (but could have missed it) though it makes perfect sense that it would since log.Panic() exists. I only have one spot where I gave myself a more detailed error message and that was after the return of parseFrontMatter() and that was only because I hit a weird issue when I originally wrote the function. I wish I had noted it down so I could have included it in the post but forgot.

I hope to fix up the front matter code to handle errors better by next week and you've given me a lot to consider which I think will help with that section and hopefully my code over all.

 

It's getting interesting ;-)

Here is my suggestion for refactoring

func splitData(fm []byte) ([][]byte, error) {}

From an outside view suddenly there is a hardcoded reference to the delimeter constant (by the way, constants always CAPSLOCK?), which is annoying to test. And difficult to change or extend.

Better to inject that dependency with:

func splitData(fm []byte, delimiter string) ([][]byte, error) {}
 

It's getting interesting ;-)

I certainly hope so! Thanks for the comment! I'm surprised I didn't notice the lowercase const. When I started I thought about allowing other styles of delimiters (***, or ___) but in the end, decided to stick with the triple-dash as Jekyll does. Since I'm not planning on changing it then there is no reason to leave it outside splitData() really. Moving it inside the function would make that function clearer, then if it turns out we need to extend it accepting the delimiter as a parameter it is probably the way to go.