import sanitize "github.com/kennygrant/sanitize"
is better written as just plain ol':
import "github.com/kennygrant/sanitize"
since the rename is redundant.
As already mentioned, it's more common to be a bit more explicit with error handling in Go and let utility functions (such as your sendRequest) return any errors for handling "higher up".
In addition to Effective Go,
I suggest looking at the Go project's Code Review Comments page.
Although the later is only what the Go Authors use and isn't necessarily more widely applicable, I find it a good starting point.
Among other things, with respect to naming it suggests (again as previously mentioned) using EventType, UserID, parsedJSON, etc instead of what you use. Also, while on the subject of naming, I personally subscribe to Russ Cox's naming philosophy which suggests i vs index, msg versus currentMessage, and e vs events[index] for identifiers used within a short span (as opposed to file or project wide identifiers).
When it comes to marshalling to/from JSON, use of struct tags allows you to not only map Go style names to JSON style names but also apply other attributes (notable things like "omitempty": FieldName string `json:"field_name,omitempty"`). In particular, without this if you tried to marshal your stucture back into JSON I believe you'd end up with your JSON fields capitalized (as Go's encoding/json package can only deal with exported fields).
The code review comments page also recommends applying caution/restraint to the use of named result parameters.
They can be useful for documentation via godoc but I try and avoid them just to save a variable initialization line in the body.
Your tastes may vary, but if you use them be careful of shadowing the named returns (e.g. with a named err error and inside an if/for having something like x, err := func()).
Instead of using something like ioutil.ReadAll to pre-read all the data I find it useful to look for "streaming" io.Reader based alternatives.
For programs that process large data inputs (e.g. reading from a large file) piece by piece this can make a big difference.
Here, encoding/json provides a Decoder type for this purpose.
In this specific case I don't know if it makes any memory/performance difference but I find the result of using it (and combining your send and parse functions) seems to simplify the code.
Back to marshalling, it can sometimes be helpful/useful to create your own thin wrapper types with custom (un)marshalling rather than limiting yourself to basic types.
For example, in the code below I've added a setime type that unmarshals from the way StackExchange API is documented to do all time/dates. You could easily add a MarshalJSON method if marshalling was also required.
The difference isn't that important here, but for example, it allows easier use of all the time.Time methods/functions (e.g. time.Since(msg.Timestamp.Time), changing the output timezone, etc.
In several places you do things like:
for i := 0; i < len(newestMessages); i++ {
var currentMessage = newestMessages[i]
…
for index := 0; index < len(events); index++ {
// use events[index]
It's more idomatic to do:
for _, msg := range newestMessages {
// use msg
…
for i := range events {
// use events[i] or
e := &events[i]
// and use e.Content, &e, etc
or even:
for _, e := range events {
// use e
In this specific case, this last form would make an extra copy (and make the e.Content = fn(e.Content) line only modify the copy).
I wouldn't worry about the extra copy here, but you could avoid it by using e := &events[i] as shown above or by changing the type to []*Events (although at the expense of producing more garbage).
Your main routine has a quit channel that doesn't get used.
If this was for future expansion, or for use if/when main was turned into a cancel-able function then that's fine.
(Although, perhaps using defer ticker.Stop() immediately after creation would be more idiomatic in that case.)
However, as-is main can be greatly simplified by removing the unused channel.
Instead of checking for a pair of explicit user ids in the middle of your code, it's cleaner to move those into either a list or a map.
In particular, using a map[userIDType]bool allows the check to be just if ignoredUsers[msg.UserID] {continue}.
Finally, for things like FKey, I prefer to put any authorization/API tokens anywhere but in hard-coded constants.
Alternatives are on the command line, in an environment variable, in a config file, or some combination of those.
Oh, and it's unclear to me what the API is, but perhaps you should be setting the "since" field in your request to either the latest timestamp or something like time.Since(latest) + fudgeDuration or somesuch.
Edit: I made this change but forgot to mention it.
You rarely need/want to call an error's Error() method.
In particular, fmt.Println(err.Error()) is a long winded way of saying fmt.Println(err) (fmt handles both fmt.Stringer, aka String(), as well as error interfaces).
I've seen others do things like panic(err.Error()) which is worse then panic(err) since although the message is the same it throws away any extra information/context (lost to someone trying to recover for example).
Here is what I came up with after applying the above.
[Also available as a git clone-able gist with changes made a few at a time so you can see some intermediate alternatives.]
package main
import (
"encoding/json"
"flag"
"fmt"
"html"
"log"
"net/http"
"net/url"
"os"
"strconv"
"time"
"github.com/kennygrant/sanitize"
)
type ChatEvent struct {
EventType int8 `json:"event_type"`
Timestamp setime `json:"time_stamp"`
Content string `json:"content"`
UserID int64 `json:"user_id"`
Username string `json:"user_name"`
MessageID int64 `json:"message_id"`
ParentID int64 `json:"parent_id"`
ShowParent bool `json:"show_parent"`
MessageEdits int8 `json:"message_edits"`
MessageStars int8 `json:"message_stars"`
}
type ChatEvents struct {
Events []ChatEvent
Time int64
Sync int64
Ms int8
}
const (
mode = "Messages"
chatURL = "http://chat.stackexchange.com/chats/8595/events"
// Never use this value directly: SE only uses it as a hint
// and might return a bit more when it feels like it
messageCount = 100
since = 0
)
var (
lastMessage time.Time
fkey = flag.String("fkey", "", `StackExchange chat "fkey"`)
ignoreUsers = map[int64]bool{
-263: true, // "Captain Obvious"
125580: true, // "Duga"
}
)
func main() {
flag.Parse()
if *fkey == "" {
fmt.Fprintln(os.Stderr, `missing required -fkey argument
Go to the chat, F12, reload, look at the /events call
and steal its fkey property`)
os.Exit(2) // 2 to match flag's exit code
}
for range time.Tick(time.Second) {
parsedJSON, err := fetchMessages()
if err != nil {
log.Println("fetching messages:", err)
// TODO, abort or backoff on repeated failures?
continue
}
newestMessages := getNewestMessages(parsedJSON.Events)
for _, msg := range newestMessages {
if ignoreUsers[msg.UserID] {
continue
}
fmt.Printf("[%v] %s: %s\n",
msg.Timestamp.Format("15:04:05"),
msg.Username,
msg.Content)
}
}
}
func fetchMessages() (ChatEvents, error) {
var parsedJSON ChatEvents
resp, err := http.PostForm(chatURL, url.Values{
"mode": {mode},
"msgCount": {strconv.Itoa(messageCount)},
"fkey": {*fkey},
"since": {strconv.Itoa(since)},
})
if err != nil {
return parsedJSON, err
}
defer resp.Body.Close()
dec := json.NewDecoder(resp.Body)
err = dec.Decode(&parsedJSON)
return parsedJSON, err
}
func getNewestMessages(events []ChatEvent) []ChatEvent {
output := make([]ChatEvent, 0)
for _, e := range events {
if e.Timestamp.After(lastMessage) {
e.Content = html.UnescapeString(sanitize.HTML(e.Content))
output = append(output, e)
lastMessage = e.Timestamp.Time
}
}
return output
}
// setime is an unmarshable StackExchange timestamp or date.
// See https://api.stackexchange.com/docs/dates
type setime struct{ time.Time }
func (t *setime) UnmarshalJSON(b []byte) error {
var seconds int64
if err := json.Unmarshal(b, &seconds); err != nil {
return err
}
t.Time = time.Unix(seconds, 0)
return nil
}
