jME source code has 62925 warnings

I added jME to eclipse via CVS and I got 62925 warnings. My eclipse has the following code warning level and a the following javadoc warning level.

I think the jME developers are qualified for guiness book of records.

I've only got 198 warnings for me so I think your Eclipse settings are freakishly anal retentive.  :stuck_out_tongue:

I'd bet most of those are due to the Javadoc warning levels…

OK, I disabled all javadoc warnings and all code warnings which are not Potential programming problems, but there are 351 warnings left.

If there are 351 "potential programming problems" then you should start posting corrections. :wink:

A large number of them are are going to be local variables and fields that are never read locally.  Among other things, there are several loaders that read information to variables that are not yet (or may never be) used.

If there are 351 "potential programming problems" then you should start posting corrections


package: com.jme.animation
class: BoneAnimation
constructor: BoneAnimation(String name, Bone bone, int numKeyframes)
error: The parameter bone is never read.
correction: change the constructor to BoneAnimation(String name, int numKeyframes), or do something useful with the parameter bone.

package: com.jme.image
class: BitmapHeader
errors:
- the field BitmapHeader.bisize is never read locally
- the field BitmapHeader.planes is never read locally
- the field BitmapHeader.compression is never read locally
- the field BitmapHeader.xpm is never read locally
- the field BitmapHeader.ypm is never read locally
- the field BitmapHeader.clrimp is never read locally
correction: don't waste memory with unused local variables, remove it or do something useful with them.

package: com.jme.image.util
class: DDSLoader
method: readData(boolean flip)
error: The parameter flip is never read.
correction: change the method to readData(), or do something useful with the parameter flip.

package: com.jme.input.controls.binding
class: MouseAxisBinding
method: convert(int value)
error: unnecessary cast from int to float
correction: change return Math.abs((float)value * 0.1f) to Math.abs(value * 0.1f)
=> if you multiply an integer with a floating point value, you will get a floating point value, so a cast to float is not necessary.

... and many more of them

These are all minor, no need for alarm.

Might be nice to do a little clean up as part of the next release perhaps ??

To be clear, we've done many such cleanups over the last year (we turned on the same eclipse warnings at NCsoft several months ago and took care of most of the big ones.)  Those that currently remain and either introduced with recent check-ins, or were unused fields and variables that do not hurt anyone by staying where they are, etc.  (Generally they are variables marking header values we don't yet respect, or fields we have to read past anyway in a particular format reader.

we turned on the same eclipse warnings at NCsoft several months ago and took care of most of the big ones
You should not turn on the warning level when you  have to do some code clean-ups, you should keep them enabled while developing.
Malformed javadoc comments or malformed javadoc tags makes jME hard to use.

To avoid these type of threads in the future maybe it’s a good idea to set up a wiki page.



It could contain the official jME Eclipse warning settings.  Then any warnings found under these settings could be fixed by all users.



Or better yet we could use PMD and set up a jME ruleset that could be put into CVS.



Eitherway we’d have a list of warnings that the developers did consider a big deal and worth fixing.



What do we think?  :slight_smile:

My suggestion is to set up the following javadoc warning level.

To assure the quality of the documentation malformed javadoc comments and missing javadoc tags should be displayed as warning everywhere.

To assure the quantity of the documentation missing javadoc comments should be displayed as warning from protected up to public, because protected is the maximum accessibility from outside.

I can agree with both sides here - in a way this thread is "much ado about nothing" as warnings are just that - warnings, not errors.  They don't "hurt" the functionality (in most cases) and can obviously be "safely" ignored by just about everyone.



However, I do think there is some real merit in setting up some standards and not accepting code that fails to meet those standards going forward - particularly when it comes to documentation.  Look, I hate documentation too, but when you are sharing your code with so many other developers of so many different and varied backgrounds and skill levels GOOD (and valid) documentation is a must.  Not fixing bad JavaDoc tags - when your IDE has already gone through and done all the work of identifying them - is just being lazy.  All programmers are lazy, or else we wouldn't be programmers, but I think setting some baselines would only improve the codebase and, as Hal suggests avoid threads with "Shocking" titles like "jME is riddled with 10,000,000 bugs!  My IDE told me so!"



I know I'm new here but that's my $.02

These definitely should be resolved I agree.  However, I think the mentality around here is primarily that of, "We've got more important things to worry about like real bugs and features".  Would you say that developer's time when given a real bug in the system should instead use that time to fix a warning that isn't going to effect the end performance of the API?  If we were out of features and there were no other bugs to worry about then this would be a big deal…not to say it's not important at all, it's just most of the developers (if not all) have very limited time to work on the API as it is and when we do work on it we're trying to add value to it instead of fixing warnings.



Does that make sense?  That being said, anyone that wants to make those fixes and submit patches is more than welcome to do so and we'll get them included.

I agree that the drive should be for real bugs and features first of all, while not completely ignoring the major glaring warnings, but also not stopping to correct every unread variable warning especially in code that reads specific structured formats that don’t currently use some values, but perhaps will in the future.



Most of these warnings are slight efficiency things (from what I can see), and I think the real warnings that should be looked at (by community volunteers), are code warnings that could lead to potential bugs.



Tools like PMD (http://pmd.sourceforge.net/), and FindBugs (http://findbugs.sourceforge.net/) are both great static code analysis tools that will pick up on potential code problems using common bug idioms (how not to code). It’ll catch things like suboptimal expressions (reduces complicated conditionals and expressions), catches lots of NullPointer type problems which plague all Java code.



Check out more bug descriptions FindBugs can discover (FindBugs Bug Descriptions)



Both these tools can be integrated into Eclipse, and also reports can be generated during an ANT build. I’ve used them both at work so maybe I can play with jME’s ANT build and try add those two reports to be generated, and I’ll post the info to the wiki so people have an idea of the potential warnings.



The problem with any warning that you can ignore and takes more effort than it’s worth is that they tend to pile up and it becomes impossible to find the relevant warnings (good feature of both those programs is you can turn off those rules you find to be harmless).



Another way of directing effort to provide more stability to jME would be to develop more unit test cases(http://junit.org/). I think there’s been a recent checkin of some new unit tests. I think jME presents an interesting challenge to traditional unit testing because a lot of what it does is mathematical, and visual in nature, so verifying those results and testing those systems could require using such common testing patterns as mock objects to simulate things like the graphical display system (DummyDisplaySystem) etc.



Also testing concurrent applications is going to present an interesting challenge, Brian Goetz has written a little about that:

http://www.theserverside.com/tt/articles/article.tss?l=TestingConcurrent



Brian’s got some other good articles on testing using a combo of static code analysis and unit tests:

http://www-128.ibm.com/developerworks/java/library/j-jtp06206.html



All Brian’s articles:

http://www.briangoetz.com/pubs.html





I really want to start tackling the Unit test development of jME but like everything and everyone else, when you’ve got other things going on in life sometimes writing unit tests take a backseat.



Update:

I added a Testing section and Unit test resources article to the articles section in the jME wiki:

http://www.jmonkeyengine.com/wiki/doku.php?id=the_articles



All you JUnit and Unit test gurus add any resources and input you’ve got I’m just getting started with this stuff.  :slight_smile:

I completely understand Darkfrog, but the problem with that mentality is that you never really do get around to fixing these things - when does ANY product ever run out of features to be added/tweaked/fixed?

I agree, and that's why we have community support to help us with such things.  There aren't very many of us developers and like I previously stated our time is limited.  We try to add the most value for our time and have to rely on community support to fill in the gaps sometimes.  Just think of how much documentation could have been written with the time it has taken to discuss the need for documentation already. :wink:



We have a Wiki to enable the community to help support jME and provide documentation.  We have the forum to help people who have immediate questions or problems that can't be resolved from the wiki.  Ideally someone like yourself that is just starting development with jME would seek answers on the wiki and anything they can't find there can be asked in here.  That is typically what people do around here.  However, what is lacking is when an answer is found they will typically just go on to develop it into their game and don't add the solution to the wiki so others can benefit from it.  If more people would do that then our documentation would be far better.

darkfrog said:

I agree, and that's why we have community support to help us with such things.
ryiden said:

we turned on the same eclipse warnings at NCsoft several months ago and took care of most of the big ones
You should not turn on the warning level when you  have to do some code clean-ups, you should keep them enabled while developing.
Malformed javadoc comments or malformed javadoc tags makes jME hard to use.


I never said we turned them off.  But to add javadocs to that warning level would currently drown out code related warnings to the point of making them invisible.  I would of course like to see javadocs be as complete and as clean as possible, but realistically I can tell you right now that I personally won't have the time to make that happen ever.  :|