debugjavaMinor
Pausing a SwingWorker with a Timer
Viewed 0 times
withswingworkertimerpausing
Problem
I made a
The twitter API will allow 180 queries every 15 minutes, and then throw an exception with code 88 which says that the rate limit was reached. My worker is initiated from a toggle button, if the button is selected and no other worker is running (not cancelled) then a new worker starts, if it gets deselected then the worker is cancelled:
Explanation of the
First off this class must have exactly one worker running at all times (thus the check in the if above). A worker may stay in the pausing while loop but he must exit immediately after the while loop is finished (through
The worker has the ability to be paused for a specific amount of time in case the API limit is reached. The worker is paused through a
The code bellow throws a fake exception to simulate making an API call that fails because of rate limiting.
If a rate limiting exception occurs and there is no countdown already running, then a
This is pretty mu
SwingWorker which is fetching tweets using the twitter4j API. The twitter API will allow 180 queries every 15 minutes, and then throw an exception with code 88 which says that the rate limit was reached. My worker is initiated from a toggle button, if the button is selected and no other worker is running (not cancelled) then a new worker starts, if it gets deselected then the worker is cancelled:
if (jStart.isSelected()) {
if (_producer == null || _producer.isDone()) {
_producer = new ProduceStatusWorker(new Query("a"), _x++);
_producer.execute();
}
} else if (_producer != null && !_producer.isDone()) {
_producer.cancel(true);
}Explanation of the
SwingWorker:First off this class must have exactly one worker running at all times (thus the check in the if above). A worker may stay in the pausing while loop but he must exit immediately after the while loop is finished (through
if(isCancelled()))The worker has the ability to be paused for a specific amount of time in case the API limit is reached. The worker is paused through a
Timer object which notifies all the ProduceStatusWorker threads when it is finished (I have a problem here, it notifies only the ones that have created a Timer object).The code bellow throws a fake exception to simulate making an API call that fails because of rate limiting.
If a rate limiting exception occurs and there is no countdown already running, then a
Timer object will be created and started. If there is a latest QueryResult then that caries the appropriate seconds-until-reset so the Timer will finish after that amount. If the latest query result is null then a fixed timer is created (this happens in the case API calls are made, then the limit is reached, the timer is created and started, then the user stops the worker and tries to start another one before the reset is over, in that case it will not be possible to know the appropriate amount).This is pretty mu
Solution
There's debug code strewn throughout your code.
This is bad. Clean it up: remove it.
Also...
Comments should describe why your code needs to do what it does. Comments that describe how your code works should only be used in complex code segments (and even then people argue you should just clean up the code instead).
So a comment like
Should be removed or improved.
Right now it's confusing the readers of the code.
"if a timer is already running" translates to
Comments that tell me how something works make me verify whether the code works that way. And this if statement is hard to understand.
Seems simple enough.
uh...
Okay...
This is confusing, why can't it say
That'd be easy to understand. But code can't NOT do something (confusing sentence). Maybe the comment should get reversed.
Yeah, that reads better.
Code version:
Be really careful with comments that explain how the code works. If you get them wrong or correct but in a confusing manner, they will detract from the quality of the code.
For a bonus, wrap it in a function:
Wrap some more things in functions and you get
And then the comment can go.
This is bad. Clean it up: remove it.
Also...
TestException? That... scares me. It needs to go. If you need to debug things, that's fine, but remove your debug code before posting on Code Review.Comments should describe why your code needs to do what it does. Comments that describe how your code works should only be used in complex code segments (and even then people argue you should just clean up the code instead).
So a comment like
// If a timer is already running then don't start another one
if (_countdown == null || !_countdown.isRunning()) {Should be removed or improved.
Right now it's confusing the readers of the code.
"if a timer is already running" translates to
(_countdown == null || !_countdown.isRunning())? Uh, yeah, maybe...Comments that tell me how something works make me verify whether the code works that way. And this if statement is hard to understand.
// If a timer is already running then don't start another oneSeems simple enough.
if (_countdown == null || !_countdown.isRunning()) {uh...
if (there is no timer OR not timer is running) {Okay...
// If a timer is already running then don't start another one
if (there is no timer OR not timer is running) {This is confusing, why can't it say
// If a timer is already running then don't start another one
if (there is a timer AND timer is running) {
don't start another one
}That'd be easy to understand. But code can't NOT do something (confusing sentence). Maybe the comment should get reversed.
// Start a new timer if there is no timer running
if (there is no timer OR not timer is running) {Yeah, that reads better.
Code version:
// Start a new timer if there is no timer running
if (_countdown == null || !_countDown.isRunning()) {Be really careful with comments that explain how the code works. If you get them wrong or correct but in a confusing manner, they will detract from the quality of the code.
For a bonus, wrap it in a function:
// Start a new timer if there is no timer running
if (!isTimerRunning()) {Wrap some more things in functions and you get
// Start a new timer if there is no timer running
if (!isTimerRunning()) {
startTimer();
}And then the comment can go.
Code Snippets
// If a timer is already running then don't start another one
if (_countdown == null || !_countdown.isRunning()) {// If a timer is already running then don't start another oneif (_countdown == null || !_countdown.isRunning()) {if (there is no timer OR not timer is running) {// If a timer is already running then don't start another one
if (there is no timer OR not timer is running) {Context
StackExchange Code Review Q#60839, answer score: 8
Revisions (0)
No revisions yet.