Accept.
Re: null - a residue from Java. I will avoid it going on.
Re: braces - I usually decide by the content of the if. If it can fit good
in one line, otherwise I break it to lines with braces. So, my definition
is not definitive. But I will take your comment and use braces. No harm
(well, four braces and four line breaks (-; )
 On Sep 14, 2013 8:08 AM, "Josh Albrecht" <notifications@github.com> wrote:

> In src/main/scala/com/codexica/s3crate/filetree/local/LinuxFileTree.scala:
>
> > @@ -11,6 +33,26 @@ import scala.concurrent.ExecutionContext
> >   * @author Josh Albrecht (joshalbrecht@gmail.com)
> >   */
> >  class LinuxFileTree(baseFolder: File, ec: ExecutionContext) extends LocalFileTree(baseFolder, ec) {
> > +
> > +  private def getPath(file: File): Path = {
> > +    //check for null parameter
> > +    if (file == null) throw new InaccessibleDataError("file parameter cannot be null", null)
>
> I think I'd actually want to omit the null check here--the semantics of
> InaccessibleDataError are that it should be thrown when the path is
> inaccessible, temporarily unavailable, or something else that happened
> because of the external environment. The point of it is to wrap all the
> exceptional situations that we can NOT fix by changing our code (but must
> be fixed by changing the environment). In the cause of a null file, that's
> entirely our fault, and it should be propagated and NOT handled, so that
> the tester/programmer/user notices and the code gets fixed.
>
> I should update the docstring to be more explicit about that.
>
> More generally, I think you should almost never check for NPEs in pure
> scala code, since nothing should ever be null. If it COULD be null, I use
> an option instead. So a NPE ALWAYS represents an unhandled error. This
> reduces boiler-plate and make the interface much clearer about what is and
> is not required.
>
> Final, minor stylistic point: I always use braces around ifs. I know this
> goes against certain scala conventions, but I like consistency. I'll
> consider changing my style because I kind of like the way your code looks :)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/joshalbrecht/s3crate/pull/1/files#r6363972>
> .
>