Skip to content
This repository was archived by the owner on May 24, 2023. It is now read-only.

retrieving.md: description of rows.Close() handling doesn't match with the code #70

Closed
jf opened this issue May 22, 2015 · 4 comments
Closed

Comments

@jf
Copy link

jf commented May 22, 2015

In Fetching Data from the Database, the explanatory text says Notice, however, that we check the error first, and only call rows.Close() if there isn't an error, in order to avoid a runtime panic.; but this "check error and close if no error" isn't there in the code:

var (
    id int
    name string
)
rows, err := db.Query("select id, name from users where id = ?", 1)
if err != nil {
    log.Fatal(err)
}
defer rows.Close()
for rows.Next() {
    err := rows.Scan(&id, &name)
    if err != nil {
        log.Fatal(err)
    }
    log.Println(id, name)
}
err = rows.Err()
if err != nil {
    log.Fatal(err)
}

I see only a defer rows.Close(). Nowhere else is rows.Close() called.

@arnehormann
Copy link
Contributor

That is perfectly ok.
log.Fatal calls os.Exit and terminates the program - so you can think of it like return regarding the control flow. By deferring rows.Close, it is called as a last instruction when the current function terminates.
If rows.Close were called directly, it would have to be added before each call to log.Fatalf or return, and at the end of the function.
Defer spares you the effort and duplication - especially concerning maintenance. Without defer, you have to remember to call it whenever you add a new way to exit the function.

@jf
Copy link
Author

jf commented May 22, 2015

I am aware of how defer works. That is not the point of this issue.

@arnehormann
Copy link
Contributor

I'm sorry if I missed something, but (comments added):

rows, err := db.Query("select id, name from users where id = ?", 1)
// check error
if err != nil {
    // error, log and terminate, no Close
    log.Fatal(err)
}
// no error, Close when function ends
defer rows.Close()

To me, that looks exactly like Notice, however, that we check the error first, and only call rows.Close() if there isn't an error, in order to avoid a runtime panic.

@jf
Copy link
Author

jf commented May 22, 2015

Ok I see. I get it now, sorry about that. I was reading that point as if it was relating to code after the rows.Next(). It just didn't fit in my mind that we would bring up a point about code that's later (You should always check for an error at the end of the for rows.Next() loop.) before we bring up a point about code that comes earlier (ie. the defer). Maybe it would make more sense to rewrite or reorder the points a bit? Thanks for the added clarification.

twoi added a commit to twoi/go-database-sql-tutorial that referenced this issue Aug 17, 2019
As I'm [not the only one](VividCortex#70) who stumbled upon this, let's just add a comment to the example code.

No logging library I have worked with goes as far as terminating the program, so many go beginners may not understand this - although I think it does make sense. (Otherwise - what's the meaning of "fatal"?)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants