top | item 27735423

(no title)

Altmetr | 4 years ago

I am the author. Could you provide a feedback about java part? There are some compromises made to keep API as similar as possible between different bindings and the main part is in c\c++ but overall I think java part is not bad

discuss

order

NwpierratorR|4 years ago

Java has different codestyle, i.e. camelCase instead of snake_case, brackets shouldn't break the line, no space before param blocks after signature, etc.

Also explicit throws is so 2000s, nowadays it would be a better approach to return class that encapsulates (Result | Error)

Etheryte|4 years ago

This is all stylistic preference which is by definition purely subjective. Just because a codebase isn't written in the same way as [big Java corp], doesn't mean it's bad.

Altmetr|4 years ago

I disagree about code style and dont think that its an issue. I know that its not the most popular style, specially in java world but this project is mostly written in c\c++. There is a meaning in keeping code style similar across different bindings, since its easier for developers.

Also, there are autoformatters for java(config for eclipse) and for c++(clang-format).

noisem4ker|4 years ago

>Also explicit throws is so 2000s, nowadays it would be a better approach to return class that encapsulates (Result | Error)

Really not sure about this. Certainly not a Java convention, in my experience.

mdaniel|4 years ago

https://github.com/brainflow-dev/brainflow/blob/4.3.2/java-p... fails to close that InputStream, which leaks file handles every time that method is invoked. Even the javadoc shows the correct usage of wrapping the InputStream in a try-with-resources block https://docs.oracle.com/javase/8/docs/api/java/nio/file/File...

The line right above that one fails to check the return type for file.delete() as that is a _request_ to delete the file and may not succeed for any number of reasons that might interest the code. If the file fails to delete, it can be retried, or one can use file.renameTo to try and move the _name_ out of the way, even if the on-disk allocation still remains. Of course, one should similarly check the result of file.renameTo as it is also a request

Swallowing exceptions is a pet peeve of mine https://github.com/brainflow-dev/brainflow/blob/4.3.2/java-p... because (among other things) it doesn't even acknowledge that an exception occurred in the error message right below it, meaning if one wished to debug _why_ that error occurred, now one must either use a debugger or recompile to include that information. The method similarly does not disclose that the result may be null, and NullPointerExceptions are my other pet peeve since unless running on a bleeding edge JVM, the error message is always "NullPointerException: null" meaning it says absolutely nothing about what it was trying to do when it fell over

Kind of related to that, the javadoc for some of the further down methods say "throws WhateverError" but not _why_ or what influence the caller has over that outcome

That's as far as I looked before closing the tab and wondering if I should mention the file handle leak, since if you only found the repo, then telling you wouldn't accomplish anything. But if it was your repo, then raising your awareness might be helpful

Then, yes, of course I agree with the sibling comments that the code style is very, very distracting, but there's no way I would have bothered to comment in HN about code style since to each his/her own so long as I don't have to work in it :-)