6
\$\begingroup\$

Trivial utility program meant to go through the list of (small) files and report file offset of the first difference between them. The goal of the code is simplicity.

Any recommendations are useful. Be brutal. I'm collecting go experience :)

package main

import (
    "fmt"
    "io/ioutil"
    "os"
)

func main() {
    if len(os.Args) < 3 {
        fmt.Println("expecting at least 2 arguments")
        return
    }

    nfi := len(os.Args) - 1

    bufs := make([][]byte, nfi)

    // read in the files
    for i, fn := range os.Args[1:] {
        fi, err := os.Open(fn)
        if err != nil {
            fmt.Printf("error opening file %s: %v", fn, err)
            return
        }
        defer fi.Close()

        bufs[i], err = ioutil.ReadAll(fi)
        if err != nil {
            fmt.Printf("error reading from file %s: %v", fn, err)
        }
    }

    // get a minimum len of all the buffers
    min := len(bufs[0])
    for _, b := range bufs[1:] {
        l := len(b)
        if l < min {
            min = l
        }
    }

    // compare values one by one
    for i := 0; i < min; i++ { // loop over offset

        v := bufs[0][i]            // a byte from first file
        for j := 1; j < nfi; j++ { // loop over the rest of files
            if v != bufs[j][i] {
                fmt.Printf("first difference at offset %d = 0x%x\n", i, i)
                return
            }
        }
    }

    fmt.Println("no differences found")
}
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

To understand your program better, I cleaned up up your code.

diff.go:

package main

import (
    "fmt"
    "io/ioutil"
    "os"
)

func main() {
    files := os.Args[1:]
    if len(files) < 2 {
        fmt.Fprintln(os.Stderr, "expecting at least 2 files")
        return
    }

    bufs := make([][]byte, len(files))
    for i, file := range files {
        var err error
        bufs[i], err = ioutil.ReadFile(file)
        if err != nil {
            fmt.Fprintln(os.Stderr, "error reading file %s: %v", file, err)
            return
        }
    }

    minLen := len(bufs[0])
    for _, b := range bufs[1:] {
        if len(b) < minLen {
            minLen = len(b)
        }
    }

    for i := 0; i < minLen; i++ { 
        v := bufs[0][i]           
        for j := 1; j < len(files); j++ { 
            if v != bufs[j][i] {
                fmt.Printf("first difference at offset %[1]d = %#[1]x\n", i)
                return
            }
        }
    }

    fmt.Println("no differences found")
}

Code should be readable. Drop references to os.Args; It's files.

For example, unreadable code and messages:

if len(os.Args) < 3 {
    fmt.Println("expecting at least 2 arguments")
    return
}

and so on.

Readable code and messages:

files := os.Args[1:]
if len(files) < 2 {
    fmt.Fprintln(os.Stderr, "expecting at least 2 files")
    return
}

and so on.

Also, error messages go to stderr.

In Go, lengths are pre-calculated. Delete

nfi := len(os.Args) - 1

nfi is simply the efficient and meaningful len(files).

Why is it so complicated to read file data?

    fi, err := os.Open(fn)
    if err != nil {
        fmt.Printf("error opening file %s: %v", fn, err)
        return
    }
    defer fi.Close()

    bufs[i], err = ioutil.ReadAll(fi)
    if err != nil {
        fmt.Printf("error reading from file %s: %v", fn, err)
    }

Note: defer fi.Close() doesn't work the way you want it to. The defers don't run until main returns.

Simplify, just ReadFile,

    var err error
    bufs[i], err = ioutil.ReadFile(file)
    if err != nil {
        fmt.Fprintln(os.Stderr, "error reading file %s: %v", file, err)
        return
    }

You wrote

    if l < min {
        min = l
    }

It's more readable to line them up

    if min > l {
        min = l
    }

I found the variable name min a little terse. I used minLen. And, again, l := len(b) is redundant. len(b) is pre-calculated.

// get a minimum len of all the buffers
min := len(bufs[0])
for _, b := range bufs[1:] {
    l := len(b)
    if l < min {
        min = l
    }
}

became

minLen := len(bufs[0])
for _, b := range bufs[1:] {
    if minLen > len(b) {
        minLen = len(b)
    }
}

You write:

fmt.Printf("first difference at offset %d = 0x%x\n", i, i)

Why 0x%x?

Why i, i?

Using the fmt package documentation:

fmt.Printf("first difference at offset %[1]d = %#[1]x\n", i)
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Thank you! Perfectly clear. Line up min > l is most surprisingly pleasant. \$\endgroup\$
    – biosckon
    Oct 12, 2018 at 20:50

Your Answer

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

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