-
Notifications
You must be signed in to change notification settings - Fork 62
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
Auto Inc/Dec on long press #27
base: master
Are you sure you want to change the base?
Conversation
These changes would allow the user to auto increment/decrement the value faster when long pressed without the need of multiple clicks. Original click functionality would remain the same.
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.
Also, add tests for all the changes you're bringing in. It's not a good idea to use handlers in code as improper handling of it can cause a thread leak.
//private boolean mAutoIncrement = false; | ||
//private boolean mAutoDecrement = false; | ||
//public int mValue; | ||
class RptUpdater implements Runnable { |
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.
It's not a good idea to declare a sub-class within a class to perform an operation. Try to separate the class out and try to pass the variables instead of using global variables
a.recycle(); | ||
} | ||
|
||
|
||
//################### Auto Increment on Long Press ######################## | ||
private boolean inc_pressed = false; |
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.
Camel Case these variables
//################### Auto Increment on Long Press ######################## | ||
private boolean inc_pressed = false; | ||
private boolean dec_pressed = false; | ||
private int _DELAY = 100; |
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.
It's not a good idea to initialize a variable with underscore in Java. Also it's ideal for constants to be declared in a separate class to hold the constants as the changes will be less.
pView.onTouchEvent(pEvent); | ||
|
||
if (inc_pressed) { | ||
if( (pEvent.getAction()==MotionEvent.ACTION_UP || pEvent.getAction()==MotionEvent.ACTION_CANCEL) |
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.
Good idea to use object.equals instead of ==
. Ideally, the snippet should be MotionEvent.ACTION_UP.equals(pEvent.getAction())
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.
Good idea to use object.equals instead of
==
. Ideally, the snippet should beMotionEvent.ACTION_UP.equals(pEvent.getAction())
it will be null safe as it is compared to a constant , but still .equals is better idea
These changes would allow the user to auto increment/decrement the value faster when long pressed without the need of multiple clicks.
Original click functionality would remain the same.