New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
errorprone :: JDKObsolete Fix #755
base: main
Are you sure you want to change the base?
Conversation
* | ||
* @return the next token in the string. | ||
* @exception NoSuchElementException if there are no more tokens in this tokenizer's string. | ||
* @see java.util.Enumeration | ||
* @see java.util.Iterator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these javadocs are now misleading as they were intended to show the behavior of this class vs the Enumeration class. Now that it's using Iterator as the base, the need for these methods may have changed.
We might want to think through the api changes here. If the method names change to conform to the new base class, perhaps we should still keep the old methods, remove the Override
tag and redirect them to the new methods that do respect the new Override
behavior. If we do want to break the class api, we should clean up the comments and the methods we no longer need.
@@ -31,7 +31,7 @@ public class MoveSpool implements Runnable { | |||
private static final Logger logger = LoggerFactory.getLogger(MoveSpool.class); | |||
|
|||
// The payload FIFO | |||
protected final LinkedList<SpoolItem> spool = new LinkedList<>(); | |||
protected final ArrayList<SpoolItem> spool = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use ArrayList
here vs ArrayDeque
? Performance isn't much of a concern based on possible sizes, so both are probably sufficient, but ArrayDeque
has a nicer interface for operating on both ends and might be closer to the intent of the spool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use
ArrayList
here vsArrayDeque
? Performance isn't much of a concern based on possible sizes, so both are probably sufficient, butArrayDeque
has a nicer interface for operating on both ends and might be closer to the intent of the spool?
Because these resolutions were auto-generated by errorprone.
@@ -28,7 +27,7 @@ public class JournaledChannelPool implements AutoCloseable { | |||
final int max; | |||
final Path directory; | |||
final String key; | |||
private final Queue<JournaledChannel> free = new LinkedList<>(); | |||
private final ArrayList<JournaledChannel> free = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment here on use of ArrayList
vs ArrayDeque
e9ca0eb
to
d219479
Compare
d219479
to
8e26f76
Compare
No description provided.