patterngoMinor
Manipulating /etc/hosts file using Go
Viewed 0 times
filehostsmanipulatingusingetc
Problem
This is my first go at a Go program that's not a variation of "Hello {$name}". It's about twice as long as my previous Python implementation, and perhaps an order of magnitude messier.
The goal is to read in /etc/hosts and combine the various entries I've made over time:
into:
Any thoughts on how to make this cleaner, better, and/or more idiomatic?
```
package main
import (
"os"
"io/ioutil"
"strings"
"sort"
)
// Because this doesn't exist?
type StringSet struct {
set map[string]bool
}
func NewStringSet() StringSet {
return StringSet{ make(map[string]bool) }
}
// Returns true if added, false if already exists
func (s StringSet) Add(str string) bool {
_, found := s.set[str]
s.set[str] = true
return !found
}
// Returns the hostnames in alphabetical order
func (s StringSet) GetHostnames() []string {
keys := make([]string, 0, len(s.set))
for key := range s.set {
keys = append(keys, key)
}
sort.Strings(keys)
return keys
}
// Because this doesn't exist?
func pop(s []string) (string, []string) {
var x string
x, s = s[0], s[1:len(s)]
return x, s
}
// Returns the lines of a file
func GetLines(filename string) []string {
raw, err := ioutil.ReadFile("/etc/hosts")
if err != nil {
panic(err)
}
return strings.Split(string(raw[:]), "\n")
}
func WriteHostsFile(outfile string, hosts map[string]StringSet) bool {
// Range outputs a random order -.-
fd, err := os.Create(outfile)
if err != nil {
panic(err)
}
defer fd.Close()
fd.WriteString("# Managed by hosts.go\n")
for k := range hosts {
names := hosts[k].GetHostnames()
line := k + " " + strings.Join(names, " ")
fd.WriteString(line + "\n
The goal is to read in /etc/hosts and combine the various entries I've made over time:
127.0.0.1 host1.test
127.0.0.1 host2.test
111.111.111.111 staging.dev otherproject.dev
127.0.0.1 localhost host.testinto:
127.0.0.1 host.test host1.test host2.test localhost
111.111.111.111 otherproject.dev staging.devAny thoughts on how to make this cleaner, better, and/or more idiomatic?
```
package main
import (
"os"
"io/ioutil"
"strings"
"sort"
)
// Because this doesn't exist?
type StringSet struct {
set map[string]bool
}
func NewStringSet() StringSet {
return StringSet{ make(map[string]bool) }
}
// Returns true if added, false if already exists
func (s StringSet) Add(str string) bool {
_, found := s.set[str]
s.set[str] = true
return !found
}
// Returns the hostnames in alphabetical order
func (s StringSet) GetHostnames() []string {
keys := make([]string, 0, len(s.set))
for key := range s.set {
keys = append(keys, key)
}
sort.Strings(keys)
return keys
}
// Because this doesn't exist?
func pop(s []string) (string, []string) {
var x string
x, s = s[0], s[1:len(s)]
return x, s
}
// Returns the lines of a file
func GetLines(filename string) []string {
raw, err := ioutil.ReadFile("/etc/hosts")
if err != nil {
panic(err)
}
return strings.Split(string(raw[:]), "\n")
}
func WriteHostsFile(outfile string, hosts map[string]StringSet) bool {
// Range outputs a random order -.-
fd, err := os.Create(outfile)
if err != nil {
panic(err)
}
defer fd.Close()
fd.WriteString("# Managed by hosts.go\n")
for k := range hosts {
names := hosts[k].GetHostnames()
line := k + " " + strings.Join(names, " ")
fd.WriteString(line + "\n
Solution
First pass on your code:
Cheers!
- Don't panic. For example, your
WriteHostsFilefunction should returnerrorinstead ofbool.
- Don't create a special struct for a string set. Just use
map[string]booland exploit zero values =) Don't be afraid to usemap[string]map[string]boolto represent your hosts.
- Your
Addfunction go away, but fyi: you are making it return a bool and never using it.
- Your
popfunction could be simplified and inlined. Just useip, parts = parts[0], parts[1:].
- Check for nil value using
foo == nilinstead oflen(foo) == 0.
strings.Split(line, " ")only works if things are separated with spaces — but indentations are a perfectly valid separator for/etc/hosts=)
- If your lines aren't too long (and you can probably assume lines from
/etc/hostsare going to be smaller than 65k chars), usebufio.Scannerto read a file line by line. See this SO answer for a full example and a discussion of caveats.
- Your
GetLinesfunction should probably be inlined, but fyi: you're silently discarding its argument.
- You probably want to use flags for the input and output paths, and output on
stdoutby default (maybe have a flag to override/etc/hosts). Outputting to a file namedhostsin the current directory is pretty unexpected.
- Instead of
for k := range hostsand then callinghosts[k], you can use directlyfor k, h := range hostsand use h.
- You don't want to write into a file line by line — what if you're overwriting the original file and your binary fails in the middle? You probably want to have
WriteHostsFilewrite into abytes.Bufferand write everything at once at the end.
- All your functions are exported (they start with an uppercase letter), but this is not a library; so you shouldn't export anything: they should all start with a lowercase letter.
- Your algorithm is not deterministic, because you're iterating over the elements of a map in
WriteHostsFile: it might be unexpected to see the file modified if all hosts are already correctly grouped. Sorting the hosts by key; or by first appearance in the original file, would solve this.
Cheers!
Context
StackExchange Code Review Q#149498, answer score: 2
Revisions (0)
No revisions yet.