<p>Hi folks,</p>
<p>I'm a rather new Go programmer, and I find myself really loving the language. I do a lot of work with geographic data and so I wrote this package to help work with WKT (Well-Known Text) files, though currently it would realistically work with any delimited text file. </p>
<p>Here it is on github: <a href="https://github.com/tomplex/wktfile" rel="nofollow">https://github.com/tomplex/wktfile</a></p>
<p>I'm coming over from primarily working with Python & PL/SQL, so I really want to be sure that I'm writing Go the idiomatic way and not getting into any bad habits. I know that this package is rather small and not incredibly helpful, but I'm mostly just testing the waters here and trying to improve my familiarity with Go before I tackle something bigger, and hopefully try and convince some folks at work to try out the language.</p>
<p>If there's a more appropriate place to post this, or this breaks any rules, please let me know and I'll be more than happy to remove it or ask somewhere else.</p>
<p>Thanks in advance!</p>
<hr/>**评论：**<br/><br/>: <pre><p>[deleted]</p></pre>eyesoftheworld4: <pre><p>Hey, thanks for pointing this out. I actually went back and forth on the placement of the <code>defer</code> here, and my reasoning was that if there's an error, but for some reason the file was actually opened anyway, and the calling program didn't bail out at the error, then the file would never actually be closed until the application exits. Is this an unreasonable assumption?</p>
<p>Also, thanks for reminding me of <code>gofmt</code>. I'll check that out right now!</p></pre>: <pre><p>[deleted]</p></pre>eyesoftheworld4: <pre><p>Oh, that makes perfect sense then. Thanks for the response!</p></pre>eyesoftheworld4: <pre><p>Do you mind if I ask a question? I'd like to make this package a bit more helpful when used for geometry processing and allow the caller to specify a handler function that will convert a specified geometry column to the geometry implementation of their choice. The "functional parameters" pattern that I've used has been really helpful to specify attributes of my <code>WKTFile</code> type, but I'm not sure if I could use it to inform this behavior as well, or if there's a more idiomatic way to do that. For a more specific example, I was thinking the caller would specify a handler function like:</p>
<pre><code>func convertWKT(wkt string) (geometry interface{}) {
geometry, err := geometrypackage.FromWKT(wkt)
if err != nil {
// handle
}
return
}
</code></pre>
<p>But I'm unsure how I could pass that function to Read() or otherwise allow the package to use it to process the data from the file. </p>
<p>Does this make sense? Or is it likely more trouble than it's worth?</p></pre>: <pre><p>[deleted]</p></pre>eyesoftheworld4: <pre><p>I will need to take a little bit to consider and decide how I want to implement what you've suggested, but I definitely think this is the right way forward. </p>
<p>Also, I want to say thanks a lot for all your thoughtful feedback. I really appreciate the time you've taken to help me.</p></pre>Jur93n: <pre><p>My question is why many of the public functions you write like this:</p>
<p>func PipeDelimited(wkt *WKTFile) error {</p>
<p>instead of:</p>
<p>func (wkt *WKTFile) PipeDelimited() error {</p>
<p>Also why is this function returning an error?</p></pre>eyesoftheworld4: <pre><p>Sure, thanks for commenting. When I was writing the <code>Read</code> function, the constructor for a <code>WKTFile</code>, I wanted to be able to pass it optional parameters, like in Python how you can do <code>def somefunction(a, optional=None):</code>. Obviously, this isn't a thing in Go, but I found this <a href="https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis" rel="nofollow">blog post</a> by Dave Cheney who uses this Functional Options idiom to provide default options for his constructors, as well as make his API easy to add features to without breaking, something that I hope to do with this package. There's another example in <a href="https://github.com/tmrts/go-patterns/blob/master/idiom/functional-options.md" rel="nofollow">this Gist</a>.</p>
<p>Finally, defining these options as methods of the <code>WKTFile</code> object itself was inferior in my opinion, as it required another step before the data was usable in the form I wanted. For example, if these were methods, the workflow would go from:</p>
<pre><code>func main() {
wkt, err := wktfile.Read("some_file.wkt", wktfile.CommaDelimited)
if err != nil {
// handle err
}
for i, row := range wkt.Rows() {
// Do stuff with data
}
</code></pre>
<p>to...</p>
<pre><code>func main() {
wkt, err := wktfile.WKTFile{FilePath:"file.wkt"}
// handle err
wkt.CommaDelimited()
wkt.Parse()
for i, row := range wkt.Rows() {
// Do stuff with data
}
</code></pre>
<p>I believe the first option has a cleaner API and is easier to read and use, though some may disagree with me.</p>
<p>Finally, you are right; these functions definitely don't need to be returning errors, as they will never cause one. I should change that, as it will remove some unnecessary error handling on my part.</p></pre>: <pre><p>[deleted]</p></pre>eyesoftheworld4: <pre><p>This is a really good point, and it did slightly bother me while looking at the godoc that those helper functions were all at the top, sort of distracting from the main point of the package. Thanks for this, I think I like this pattern better, too.</p></pre>allhatenocattle: <pre><p>Overall, pretty good.<br/>
One small nit, in your README, when finding an error, you should return or exit</p>
<pre><code>if err != nil {
fmt.Println("error reading file")
// return or exit here
}
</code></pre></pre>eyesoftheworld4: <pre><p>Sure, good point. Even if it's sample code, no reason to give it a way to potentially fail! Thanks, I modified it.</p></pre>
