# Extract title with regex

I'm quite new to Go and I feel like this code could be smaller and cleaner.

I would love any suggestions and/or hints about mistakes and conventional go things!

func getBookTitle(client *http.Client) {
rsp, err := client.Get(bookSite)
if err != nil {
panic(err)
}

//Get div with title
regTitle := regexp.MustCompile("(<div class=\"dotd-title\">)[\n+\\s]*(<h2>)[a-zA-Z–\\-\n\\s:]*(</h2>)[\n+\\sdd]*(</div>)")
//remove linebreaks regex
regFormatTitle := regexp.MustCompile("[\r\n]*")
//apply regex
title := regFormatTitle.ReplaceAllString(string(regTitle.Find(html)),"")
//Remove remove html tags and remove whitespaces
title =  strings.TrimSpace(title[strings.Index(title,"<h2>")+len("<h2>"):strings.Index(title,"</h2>")])

fmt.Printf("Book title:%s\n",title)

rsp.Body.Close()
}


### Go practices

It's recommended to close things using defer as early as possible, so that you won't forget them later. For example you have:

html, _ := ioutil.ReadAll(rsp.Body)

// some code

defer rsp.Body.Close()


Using defer, you should write like this instead:

html, _ := ioutil.ReadAll(rsp.Body)
defer rsp.Body.Close()

// some code


Another thing, it's recommended to not ignore errors. This example ignores the err value returned by ioutil.ReadAll, and that may lead to crashes later in the program.

### Single responsibility principle

It's recommended for functions to do just one thing. The getBookTitle function does do many things:

• Download a web page from bookSite (a free variable defined elsewhere)
• Read content from a stream
• Extract title from an HTML string

The name implies only the 3rd task. It would be better if it did only that, and the other tasks should be performed in other functions.

### Strange regex

This regex looks strange:

(<div class=\"dotd-title\">)[\n+\\s]*(<h2>)[a-zA-Z–\\-\n\\s:]*(</h2>)[\n+\\sdd]*(</div>)


For several reasons:

• [\n+\\s] is odd. \\s already includes \n. You could write simpler as [+\\s]. But are you really expecting a literal + between the starting <div ...> and <h2>? I seriously doubt it. In which case the expression could be reduced to just \\s.

• [\n+\\sdd] is similarly odd. Again, the \n is unnecessary. Two d are also unnecessary, and if there are literal d between </h2> and </div>, that would be invalid HTML.

• Why the groupings with (...)? Those parentheses don't serve any purpose in this program.

All in all, it seems the regex is mistakenly over-complicated, this simpler regex is probably good enough and much more clear:

<div class=\"dotd-title\">\\s*<h2>[a-zA-Z–\\s:]*</h2>\\s*</div>


janos gave a good answer. I would add one minor thing and one major thing:

1. Minor: don't panic. This is encapsulated in a function (that, indeed, does too many things) so the function should return an error if something bad is likely to happen; and you should let the caller deal with the error. Furthermore, log.Exitf is almost always what you want rather than panic.
2. Major: don't use a regexp to parse HTML. Seriously, don't. Consider using the html Go library instead.
• About 2. Even though I don't want to reconstruct any thing? I know how it is build so shouldn't it have the same error chance like when I check for a div with the id "dotd-title"? I'm basically matching in the first place. – Nordiii Apr 1 '17 at 12:05
• Here is stated that it's sometimes appropriate to parse a limited, known set of HTML which would apply on this case or am I wrong? – Nordiii Apr 1 '17 at 12:16
• The important bit in that answer is "This was a limited, one-time job." If that's your case, and you're going to throw this code away afterwards and never use it again, then whatever does the job works =) But since you wanted your code reviewed, and encapsulated this logic in a function, it doesn't look like it's your case. – Ted Apr 1 '17 at 20:12