Skip to content

Conversation

@emmanuelkehinde
Copy link
Contributor

Considering what most users have gotten used to, allowing them to enter their expiry date in text will provide a better user experience.

@biodunalfet
Copy link
Contributor

Looks good. Will test and merge soon. Thanks

public void afterTextChanged(Editable editable) {
String input = editable.toString();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MM/yy");
Calendar calendar = Calendar.getInstance();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one man, learnt something new 😄

You should consider creating a static Calendar as cost of calling Calendar.getInstance every time text is changed is quite high see see

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh...true.

I will fix that. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's updated now.

@Override
public void afterTextChanged(Editable editable) {
String input = editable.toString();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MM/yy");
Copy link
Contributor

@biodunalfet biodunalfet Mar 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're creating an instance of simpleDateFormat each time the text changes which is expensive. Try to create it once in the ExpiryWatcher()'s constructor and use that instance in line 860 instead just like you did for your calendar variable. Neat work by the way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh true. Correcting that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE.

@biodunalfet biodunalfet merged commit f2e7b14 into Flutterwave:master Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants