Javadoc #7531
Javadoc #7531
Conversation
Adding just javadocs to explain it a bit better
Update Javadocs to explain stuff better
Javadocs UwU
Javadoc again and again
Javadocs
|
I have added these to help our teacher what's happening, because we asked for t 10000 heir help and they were just lost. So I understand that less might be better but I believe that it is a bit more clearer with such comments. |
There was a problem hiding this comment.
I think some of the javadoc additions are fine, also the override annotations.
However, javadoc on override methods is in my opinion counter-productive. If there is something special to note, it should be noted in the interface.
There are also a lot of inline comments that I would consider unnecessary
| public abstract class AbstractGraphics implements Graphics { | ||
|
|
||
| /** Delegates to {@link #getDeltaTime()} for the actual time calculation. |
There was a problem hiding this comment.
If a method already has javadocs on the super method, it doesn't need new javadocs.
| root.align(Align.left | Align.top); | ||
| stage.addActor(root); | ||
| root.setFillParent(true); // Make the table fill the entire stage | ||
| root.align(Align.left | Align.top); // Align the table's contents to the top-left |
There was a problem hiding this comment.
There are a lot of comments like this, where the code is 100% self explanatory.
Deletion of javadoc because super method already does
There was a problem hiding this comment.
Sorry to be blunt, but I don't like this at all. There's a lot of important things that are undocumented here, and a lot of unimportant things that are vaguely documented. Tests don't need docs, at all, unless they are part of a public API or you need to tell something to the reader. If the reader needs to know something in a test, they are probably reading the source.
I don't think your teacher will learn anything new about libGDX via these comments. I'm beginning to wonder if I could teach a libGDX class, given that I lack even a Bachelor's degree, but maybe know some things already... Not like a generic Bachelor's degree actually indicates much for job skills. Some might!
| public void clear () { | ||
| processors.clear(); | ||
| } | ||
|
|
||
| /** Empty the list of processors and implement a new list of processors |
There was a problem hiding this comment.
OK... this would be a big review.
This line isn't even right. Like, the only part that's correct is that it empties the... well, not a list... of processors. And like the rest of the PR, there's no benefit to @param processors, we can see that immediately already. It might help to know it takes 0 or more InputProcessor items as varargs or in an array! Just knowing its name doesn't tell me that.
| public void addProcessor (InputProcessor processor) { | ||
| if (processor == null) throw new NullPointerException("processor cannot be null"); | ||
| processors.add(processor); | ||
| } | ||
|
|
||
| /** Remove a processor from the list of processors |
There was a problem hiding this comment.
Would be nice to know it's removed by identity, not by value.
There was a problem hiding this comment.
I don't think any of the tests need JavaDocs. They aren't exposed to users to call... Who would be reading them? Why wouldn't they be reading the source?
There was a problem hiding this comment.
I feel it's much easier to read the code and understand it with comments and javadocs instead of looking for the source
Super has already javadocs
|
I expect Tommy will still be caught up on the "list" wording for the I'm still not a fan of some of the wording. "This test uses libGDX"... well duh. "false else" is a strange order, probably best rewrite the whole comment. |
|
It would be best to leave code changes out of this pull request, but I wouldn't mind the following minor changes to
|
Align.left | Align.top can be simplified to Align.topLeft. ScreenUtils.clear(1, 0, 0, 1) is horrible on the eyes. I would prefer something more neutral such as ScreenUtils.clear(Color.GRAY).
Pull request of javadocs I have added