Add a package to abstract SQL operations #18
Conversation
The data package should improve code-reuse by bundling together the SQL operations into a common package. It will be marginally less efficient than using SQL directly, but it will enforce best practices, remove duplication and prevent mistakes from broken queries. #7 is the associated issue. To begin with, this updates termine to use the new package. It also refactors termine a bit. Fixes #3
|
|
||
| var ( | ||
| driver = flag.String("driver", "postgres", "Der benutzte sql-Treiber") | ||
| connect = flag.String("connect", "dbname=nnev user=anon host=/var/run/postgresql sslmode=disable", "Die Verbindusgsspezifikation") |
There was a problem hiding this comment.
I think we should be consistent with the row length. Either be as long as it needs to be or not using 80 chars as a line-break-reference (compare L.25). I'd go for "as long as it is needed as long as it is reasonable".
There was a problem hiding this comment.
My comments are at 80 characters because that's what gq does. As such, this provides the point of minimum care, use gq for commends (as gofmt doesn't split them and they become unreasonably long) and gofmt for everything else. Doing a manual "is this too long or not" evaluation just seems tedious to me. Also, having basically any rules for line lengths means that at some point arguments will start about what "unreasonably long" really means :)
There was a problem hiding this comment.
ok, automated comment-formatting is reasonable. just seemed strange having this one long line, but comments splittet after 75 chars.
| t *Termin | ||
| } | ||
|
|
||
| // Next returns whether there is another element. |
There was a problem hiding this comment.
"... and changes pointer to the next element." something like that. I's an iterator after all, without Next, you don't get the next element.
|
|
||
| // Err returns the last error that occured when reading. It must be called | ||
| // after being done with the iterator. | ||
| func (it *TerminIterator) Err() error { |
There was a problem hiding this comment.
Do we want to call that Err() or rather something like Close()?
| // One is a convenience method for queries that expect exactly one result. It | ||
| // returns an error if there is not exactly one result. Must be the only method | ||
| // being called. | ||
| func (it *TerminIterator) One() (*Termin, error) { |
There was a problem hiding this comment.
Why having One() at all if we have First()?
There was a problem hiding this comment.
They serve different purposes (so far, One doesn't serve any, though). One is useful for "I am searching for a specific meeting", whereas First is useful for e.g. "I want the next meeting". In the former case, using First would potentially hide bugs in the query or unexpected data.
I'm not married to it, though. If you want, I can remove it until the need is demonstrated.
There was a problem hiding this comment.
Good point. No harm in keeping it then.
| Scan(dest ...interface{}) error | ||
| } | ||
|
|
||
| // row is an abstraction over the data elements to share code. |
There was a problem hiding this comment.
docu-comment must be updated as well
The data package should improve code-reuse by bundling together the SQL
operations into a common package. It will be marginally less efficient
than using SQL directly, but it will enforce best practices, remove
duplication and prevent mistakes from broken queries. #7 is the
associated issue.
To begin with, this updates termine to use the new package. It also
refactors termine a bit.
Fixes #3