Fact: String.split is error prone and slower.
> without interpreting its argument as regex
In JDK, String.split actually splits w/o regex if your separator is a one non-regexp char, but it's an implementation detail...
IMHO, all String methods that take regexp String should be deprecated and replaced with a new version that takes explicit java.util.regex.Pattern
param, e.g.: String.replaceAll(String, String)
-> String.replaceAll(Pattern, String)
(or even better: String.replaceAllSecondParamIsNOTWhatYouThink(Pattern, String)
- not a joke - replaceAll is often used incorrectly)
And if you run something like findbugs or any static analyzer, it'll throw you warnings for "nullables" who aren't being handled when null or used without being checked for null.
Yeah sort of bad. Findbugs has a rule to try to catch this but I think it will only pick up hardcoded uses when generally I'd like it to complain about any use of this constructor - http://findbugs.sourceforge.net/bugDescriptions.html#DMI_BIGDECIMAL_CONSTRUCTED_FROM_DOUBLE
Other gotcha with BigDecimal I ran into recently is equals checks that the number is the same AND the scale so 1.23 is not equal to 1.230 - have to use compareTo for that.
Yeah this seems a bit odd. > We (at Hazelcast) struggled/wrestled with an operation timeout bug for hours, days and weeks.
This is like day one condition variable stuff. You don't learn how to use them without learning this. I would think that anyone familiar with multithreading would stare pretty hard at a wait that wasn't surrounded by the proper loop. In fact I wouldn't be surprised if static analysis tools exist that would catch this...
EDIT: Yep, here it is http://findbugs.sourceforge.net/bugDescriptions.html#WA_NOT_IN_LOOP
> Really I'm more interested in if there is a better way to do any particular method/class.
As a general style guideline, I'd recommend reducing the visibility as much as possible. E.g. make all your fields private, if you can.
From there, there are a couple of fields which I think would be better as local variables, for example "rankCounter" in Table, but that might be a complicated refactoring to pull off.
Finally, some of your methods are pretty long, and thus difficult to follow, e.g. Table.dealerChoicesAndPayouts(), which you might be able to fix by breaking into smaller methods each with a smaller responsibility.
You may also consider using a tool like FindBugs and run it on your program: http://findbugs.sourceforge.net/
I believe the task is to iterate over the keyset, and count the size of the list in the values.
List<Position> list = new LinkedList<>(); list.add(foo1); list.add(foo2); map.put("foo", list); list = new LinkedList<>(); list.add(bar1); map.add("bar", list);
System.out.println(most());
This would then print out "bar" because the size of the associated list is 1 (compare with the list of size 2 that is at "foo").
Aside, you're probably supposed to use keySet()
, and get(key)
. If you're not, glance at entrySet() (javadoc.
for (Entry<String, List<Position>> entry : map.entrySet()) {
This then gives you access to entry.getValue()
which returns the value with the associated key (accessible at entry.getKey()). Not an issue for academic work, but if you're dealing with large maps where you are going to be doing a 'get()' for each and every key in the map, the entrySet is more efficient. Findbugs has a warning about it:
> "Inefficient use of keySet iterator instead of entrySet iterator This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup."
Run your code through a static code analysis tool like FindBugs. It will pick up on things like using a StirngBuilder in a loop that /u/sadjava mentioned. Things that aren't technically wrong but are a bad practice. It also gives you a chance to read up on a subject when you find an error.
> StyleCop.
Gah, no, just don't be frickin' a whitespace Nazis to start with.
A couple of months ago I prepared and submitted a patch to a small, one-person open source project that used one of these code style enforcement tools. It rejected a lot of my code because in some places I deliberately used two consecutive blank lines in order to provide some visual separation between logical sections of a class. I.e., it was actually punishing me for trying to format the code to make it easier to read.
Software quality tools that focus on the meaning of code, like Findbugs for Java, are better. They do have their problems, however—for example, Findbugs' nullability logic is broken.
I don't have a changetip account.
Don't worry 'bout it. Helping people is fun and that's why I'm here. :-)
If you're interested in static code analysis, check out FindBugs to check against a slew of different bug categories and Checkstyle for coding style.
EDIT: I appreciate the gesture, though. You are kind!
Optional will stop being null when Valhalla finally gets done, as it is part of the class set to be elected into value classes.
For the time being,
https://wiki.eclipse.org/JDT_Core/Null_Analysis
Just look at the rules: https://pmd.github.io/latest/pmd_rules_java.html http://findbugs.sourceforge.net/bugDescriptions.html
Sonarqube is $$$ and doesn't publish a list, but it's generally considered "worth it" compared to PMD/Findbugs if that gives you any idea how good analysis is for mature languages.
For worst practices search and static code analysis you can use something like findbubs(http://findbugs.sourceforge.net/). Add it to the project and run analysis, it will show some problems which is mostly touching bad implementation practices and basic language misuse, such as using thread unsafe static variables for example.
You can google for the following DRY, LSP, SRP, DIP, OCP. It's very basic principles which is applied to (almost) any language(definitely works for Java), touching overall code structure and architecture.
Any type of wheel reinventing, if not necessary. Frameworks re-implementation, just because we can, working with low level API's without real reason - especially within the areas which operate streams and connections, which need to be carefully handled, closed and deallocated. I.e. it's OK to maintain JVM modifications if you need effective memory operations for enterprise multi-threaded application with really high load, where you need to manipulate heap and move it to slow storage while threads is waiting, but absolutely not OK to write your own web server for small company web portal with hundred users at most.
Rest depends on technology stack, frameworks, decisions made by architect. For example sometimes DRY can be willingly ignored for critical modules to make its code isolated and fully independent.
Note: I make every effort to programatically figure out if links originally posted to Reddit are still good, but it's difficult.
If the original URL doesn't work, or has been replaced with something else, please help out by searching the Wayback Machine for the URL and posting a contemporary link if you find one. There's also a Chrome Extension which makes this process easy.
There's probably already an optional warning from compiler available, or at the very least one for FindBugs.
In fact, yup, here it is:
http://findbugs.sourceforge.net/bugDescriptions.html#DMI_BIGDECIMAL_CONSTRUCTED_FROM_DOUBLE
Without knowing anything about your setup, I would guess that looks like a FindBugs error code:
http://findbugs.sourceforge.net/bugDescriptions.html#DLS_DEAD_LOCAL_STORE
which says
> Note that Sun's javac compiler often generates dead stores for final local variables. Because FindBugs is a bytecode-based tool, there is no easy way to eliminate these false positives.
And that about fits your code - negativeRadCircle is a final local variable, so probably just a false positive. It might go away if you make it not final, but that's just a complete guess.
It looks like you weren't the only one to have this exact problem:
Thanks for the help, I can't find details on exactly what features there are though (another example of things being hard to find for Ada on the internet I suppose :/). I expect there would be a site listing the features of the static analyzers like this one for Java.
It's definitely better than nothing, but there's still plenty of cases where that won't catch errors. IO is a good example of that:
public static void main(String[] args) throws IOException { FileReader foo = new FileReader("x"); foo.read(); }
this'll compile just fine and won't give you any warnings, yet you could get an error trying to open and read the file, and there's nothing there to force you to close it and clean up when you're done.
Tools like FindBugs definitely help, and you should be using them if you're not, but it's certainly not as clean as what happens in Haskell world.
Likewise, thanks for the MonoDevelop feature roundup. I haven't used it extensively, so I wasn't aware of some of those. It does seem like for the most part it's on par. Another thing I forgot to mention that's really nice in Eclipse is FindBugs plugin. FindBugs can do very thorough code analysis and find all kinds of code smells. I'm not sure if there's anything equivalent for C#.