-
Notifications
You must be signed in to change notification settings - Fork 0
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
Builder support: MON-10, MON-11, MON-17 #1
base: master
Are you sure you want to change the base?
Conversation
@@ -156,17 +160,204 @@ public static String asCardBrand(@Nullable String possibleCardType) { | |||
* @param expYear the expiry year | |||
* @param cvc the CVC code | |||
*/ | |||
public Card( | |||
private Card( |
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.
- Revert to public modifier with
@deprecated
- Deprecation message should be something like: use Card.create
- Also note that public constructor will be removed in future, in future versions
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.
@EnilPajic I'm interested in your opinion as well
@@ -112,9 +113,12 @@ public String paymentMethodType() { | |||
private Integer expMonth; | |||
private Integer expYear; | |||
private boolean tokenizePan; | |||
private Map<String, Object> metaData; |
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.
Rename this field to 'data', meta is more data about entity itself. Data pushed to this field are more additional/non card data, not info about card itself.
final Integer expMonth, | ||
final Integer expYear, | ||
final String cvc, | ||
Map<String, Object> metaData) { |
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.
Rename every occurrence of 'metaData' to 'data'
return this; | ||
} | ||
|
||
public Object getCountry() { |
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.
You should not design api in a way:
- field(TYPE)
- Object getField()
If field type is clearly set in setter same type should be as return type.
getCountry should return String not Object. Same stands for any other field. Also add @Nullable
annotation (from android-x package)
|
||
import java.util.Map; | ||
|
||
public enum AdditionalData { |
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.
- additional data should be package local, not public
- field names should be constants within same file, package local visible
- Use field_name const in builder as well
} | ||
final String value = (String) fieldValue; | ||
return value.matches(validationData) && value.length() >= minLength && value.length() <= maxLength; | ||
case META_DATA_VALIDATION: |
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.
Rename META_DATA_VALIDATION to MAP_SIZE_VALIDATION
No description provided.