<h4><a href="https://github.com/cwramsey/file_finder">file_finder</a></h4>
<p>I just finished writing a (very) short tool in Go to help get some of the basics in the language.</p>
<p>It's a simple cli tool that (can recursively) find files matching a string.</p>
<p>It's a whopping 74 lines of code, including plenty of spacing for readability.</p>
<p>I'd love to hear what's absolutely terrible and what could be done more <code>Go-like</code>. The logic isn't perfect, but for a little tool I threw together to grasp the basics of the language, it'll do for now. That said, I'm mainly looking for your thoughts on how I can write better Go with this.</p>
<p>For example, I've read something about <code>File.ReadDir</code> might be better than <code>ioutil.ReadDir</code>?</p>
<p>Anyway, thanks in advance for taking a peek!</p>
<hr/>**评论：**<br/><br/>nstratos: <pre><p>First of all, welcome to Go and it's community. I hope you will enjoy writing the language, keep learning and having fun!</p>
<p>As you have already mentioned, this is a little program so most of the things I am going to mention are nitpicks. Nevertheless I hope they will help you learn more about the language and it's philosophy. </p>
<ul>
<li>Prefer <a href="https://talks.golang.org/2014/names.slide#7">shorter names</a> for your local variables. </li>
<li>While this is not exactly what the proverb means: "<a href="https://youtu.be/PAAkCSZUG1c?t=6m25s">Make the zero value useful</a>" is what comes to mind. </li>
</ul>
<p>You can delete those 3 lines:</p>
<pre><code>if *directory == "" {
*directory = "./"
}
</code></pre>
<p>if you use a default value as the second argument of <code>flag.String</code>:</p>
<pre><code>directory := flag.String("dir", ".", "Directory you'd like to search in. ex: /var/www/")
</code></pre>
<ul>
<li><p>While <a href="https://github.com/golang/lint">golint</a> will not complain, it's good if you have a <a href="http://blog.golang.org/godoc-documenting-go-code">doc comment</a> describing your command above <code>package main</code> so that if someone stumbles upon your program in <a href="https://godoc.org/">GoDoc</a>, they can learn what it is about.</p></li>
<li><p>Try to keep your program portable. Lines like <code>directory+"/"+thisItem.Name()</code> can be problematic. What if you try to run this on windows that uses "\" as a path separator? It's a good idea to use the <a href="https://golang.org/pkg/path/filepath/">filepath package</a> for those cases and do something like: <code>filepath.Join(directory, thisItem.Name())</code>which will ensure portability among the platforms that <a href="https://golang.org/doc/install/source#environment">Go supports</a>.</p></li>
<li><p>It is a good practice to extract as much of your logic as you can, in a separate package. The functions <code>search</code> and <code>searchRecursively</code> are good candidates.</p></li>
<li><p>Extracting your logic in a separate package has increased benefits like making your code <em>reusable</em> and <em>easier to maintain</em> and also allows you to write <a href="https://golang.org/pkg/testing/">unit tests, benchmarks and examples</a> easier. Good reads: <a href="https://golang.org/doc/code.html">How to Write Go Code</a>, <a href="https://golang.org/doc/effective_go.html">Effective Go</a>, <a href="https://github.com/golang/go/wiki/TableDrivenTests">Table Driven Tests</a>, <a href="https://blog.golang.org/cover">The cover story</a></p></li>
<li><p>Something important is that you should prefer to use interfaces (like <code>io.Reader</code>) as arguments in your functions. Think about a function that takes <code>path string</code> as argument and then inside it opens the file belonging to that path in order to perform some kind of logic on that file. You need to ask yourself: "Is it really the responsibility of that function to open the file?" In your case, the answer is probably no as the purpose of your functions is to search, not open files. Instead you could let <code>main</code> (or another function) handle the opening/reading and just keep the core search logic in the search functions. This will also allow you to:</p>
<ul>
<li>write unit tests for you function logic much easier. </li>
<li>make your functions more powerful and generic.</li>
</ul></li>
<li><p>It's always good to think about error handling. Your functions could possibly return an <code>error</code> as a second argument. For example, what if <code>os.Chdir(directory)</code> fails? Sure it is unlikely but errors are a big part of programming and you should take good care of them. Go has a slightly different philosophy when it comes to errors than other languages. Good reads: <a href="http://blog.golang.org/error-handling-and-go">Error handling and Go</a>, <a href="https://blog.golang.org/errors-are-values">Errors are values</a></p></li>
<li><p>Truly a technicality but <a href="http://tldp.org/LDP/abs/html/exitcodes.html">traditionally</a> exit code 2 is used when the user uses wrong options. You can see that a lot in Go standard library code like in <a href="https://github.com/golang/go/blob/master/src/cmd/gofmt/gofmt.go#L55-L59">gofmt</a>.</p></li>
<li><p>You can also use a <code>.gitignore</code> file so that you can ignore the <code>file_finder</code> binary file and not commit it in your repository. You can distribute your binary using <a href="https://help.github.com/articles/creating-releases/">GitHub releases</a> and Go developers can always use <code>go get</code>.</p></li>
<li><p>Have a License for your public repository.</p></li>
</ul>
<p>There might be other things you can improve but those are the ones that I noticed. Happy hacking!</p></pre>shadowycoder: <pre><p>Hey, thanks! This is exactly the kind of feedback I was looking for. I really appreciate you taking the time to write this out. <a href="https://media.giphy.com/media/MpiWetOCN5Sx2/giphy.gif">+1</a></p></pre>9eb5: <pre><p>Why not just use filepath.Walk instead of recursing into directories manually?</p></pre>shadowycoder: <pre><p>Only because I didn't know it existed yet. Thanks for the tip!</p></pre>
