2

I am just beginning to learn Go, and have made a function which parses markdown files with a header, containing some metadata (the files are blog posts).

here is an example:

---
Some title goes here
19 September 2012
---
This is some content, read it.

I've written this function, which works, but I feel it's quite verbose and messy, I've had a look at the various strings packages, but I don't know enough about Go and it's best practices to know what I should be doing differently, if I could get some tips to clean this up, I would appreciate it. (also, I know that i shouldn't be neglecting that error).

type Post struct {
    Title string
    Date string
    Body string
}
func loadPost(title string) *Post {
    filename := title + ".md"
    file, _ := ioutil.ReadFile("posts/" + filename)
    fileString := string(file)
    str := strings.Split(fileString, "---")
    meta := strings.Split(str[1], "\n")
    title = meta[1]
    date := meta[2]
    body := str[2]
    return &Post{Title: title, Date: date, Body: body}
}
3
  • 2
    A side note: blackfriday is a complete, native Go parser for Markdown.
    – jimt
    Sep 19, 2012 at 22:57
  • Thanks, I had been using a different md parser (github.com/hoisie/mustache) but blackfriday looks a bit more polished.
    – Zen
    Sep 19, 2012 at 23:11
  • Don't ignore the error from ioutil.ReadFile! Return it, log it or panic - ignoring it will cause difficult to debug problems in the future. Sep 20, 2012 at 15:40

3 Answers 3

4

I think it's not bad. A couple of suggestions:

  • The hard-coded slash in "posts/" is platform-dependent. You can use path/filepath.Join to avoid that.
  • There is bytes.Split, so you don't need the string(file).
  • You can create the Post without repeating the fields: &Post{title, date, body}

Alternatively, you could find out where the body starts with LastIndex(s, "--") and use that to index the file contents accordingly. This avoids the allocation of using Split.

const sep = "--"

func loadPost(content string) *Post {
    sepLength := len(sep)

    i := strings.LastIndex(content, sep)
    headers := content[sepLength:i]
    body := content[i+sepLength+1:]

    meta := strings.Split(headers, "\n")

    return &Post{meta[1], meta[2], body}
}
1
  • I don't think that LastIndex will be of use here, as it's pretty greedy. There may be more occurances of --- in the text. But +1 for splitting the file read from parsing the content.
    – nemo
    Sep 19, 2012 at 22:55
2

I agree that it's not bad. I'll add a couple of other ideas.

  • As Thomas showed, you don't need the intermediate variables title date and body. Try though,

    return &Post{
        Title: meta[1],
        Date: meta[2],
        Body: body,
    }
    

    It's true that you can leave the field names out, but I sometimes like them to keep the code self-documenting. (I think go vet likes them too.)

  • I fuss over strings versus byte slices, but probably more than I should. Since you're reading the file in one gulp, you probably don't need to worry about this. Converting everything to one big string and then slicing up the string is a handy way of doing things, just remember that you're pinning the entire string in memory if you keep any part of it. If your files are large or you have lots of them and you only end up keeping, say, the meta for most of them, this might not be the way to go.

  • There's just one blog entry per file? If so, I think I'll propose a variant of Thomas's suggestion. Verify the first bytes are --- (or your file is corrupt), then use strings.Index(fileString[3:], "---"). Split is more appropriate when you have an unknown number of segments. In your case you're just looking for that single separator after the meta. Index will find it after searching the meta and be done, without searching through the whole body. (And anyway, what if the body contained the string "---"?)

  • Finally, some people would use regular expressions for this. I still haven't warmed up to regular expressions, but anyway, it's another approach.

1

Sonia has some great suggestions. Below is my take which accounts for problems you might encounter when parsing the header.

http://play.golang.org/p/w-XYyhPj9n

package main

import (
    "fmt"
    "strings"
)

const sep = "---"

type parseError struct {
    msg string
}

func (e *parseError) Error() string {
    return e.msg
}

func parse(s string) (header []string, content string, err error) {
    if !strings.HasPrefix(s, sep) {
        return header, content, &parseError{"content does not start with `---`!"}
    }
    arr := strings.SplitN(s, sep, 3)
    if len(arr) < 3 {
        return header, content, &parseError{"header was not terminated with `---`!"}
    }
    header = strings.Split(strings.TrimSpace(arr[1]), "\n")
    content = strings.TrimSpace(arr[2])
    return header, content, nil
}

func main() {

    //
    f := `---
Some title goes here
19 September 2012
---
This is some content, read it. --Anonymous`

    header, content, err := parse(f)
    if err != nil {
        panic(err)
    }

    for i, val := range header {
        fmt.Println(i, val)
    }
    fmt.Println("---")
    fmt.Println(content)

    //
    f = `---
Some title goes here
19 September 2012
This is some content, read it.`

    _, _, err = parse(f)
    fmt.Println("Error:", err)

    //
    f = `
Some title goes here
19 September 2012
---
This is some content, read it.`

    _, _, err = parse(f)
    fmt.Println("Error:", err)
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.