Your code is very clear and readable in general. I came up with a very nitpicky list of style suggestions :)
A few comments on main()
time.Duration(1000) * time.Millisecond
can be written as just1000 * time.Millisecond
- Call the cancel function from
context.WithTimeout
, otherwise your program has a resource leak (not that it matters from main, but still).ctx, cancel := context.WithTimeout(...) defer cancel()
- Use
ctx
consistently, there are some places inmain
where it refers tocontext.Background()
instead. - Handle the error from
os.Create
, or your code will be impossible to debug when it fails. - Similarly, you probably want to print an error if
urlHolder.WriteTo
fails. - I would also suggest making a helper function for creating the XML file and writing to it, in particular if you ever change the file naming pattern you would need to change it in two places.
- I think there's no need for a goroutine in main? You can just run it directly unless I'm missing something.
A few things on urlholder.go
- Time
time.Time
type is almost always used as a value and not as a pointer. TheURL.LastMod
field should be atime.Time
, not a*time.Time
. - This is subjective, but
UrlMap
should be spelled asURLMap
. - No need to
make
a slice of size zero, just usenil
. UrlMap.Add
should return an error instead of panicing.- The
UrlMap.Add
method uses a different method of serialization thanWriteTo
, so the byte count may not be correct. I would suggest callingWriteTo
from insideAdd
, as followsfunc (m *UrlMap) Add(u *URL) int { s.URLs = append(s.URLs, u) var out strings.Builder m.WriteTo(&out) return len(out.String()) }
I'm not sure what the point of CounterWriter
is when the result of urlHolder.WriteTo
is ignored anyway.
And since you asked about interfaces, there's no need to use an interface here. In general you should only refactor to use interfaces when you already have two implementations in mind. Using an interface preemptively for a "cleaner" design is an example of YAGNI: you ain't gonna need it.