Skip to content
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

Deprecation and refactoring proposal #200

Closed
10 tasks done
davideas opened this issue Oct 14, 2016 · 0 comments
Closed
10 tasks done

Deprecation and refactoring proposal #200

davideas opened this issue Oct 14, 2016 · 0 comments

Comments

@davideas
Copy link
Owner

davideas commented Oct 14, 2016

I am going to refine a bit the library with some deprecations and refactoring. Please add a your comment if you are against a modification, explain why you would like to keep it and in which use case you are using it.

Also, you can propose a modification to the existent methods, always explain your reasons and the use case application.

Note: All deprecated methods are going to be removed by the publishing of the final release 5.0.0.

Refactoring

  • FlexibleAdapter.java
// Just a better name
initializeListeners(@Nullable Object listener); to addListener(@Nullable Object listener);

// Just a better name
getRelativePosition(@NonNull T child); to getSubPositionOf(@NonNull T child);

// Just a better name (dual of the setter)
// This method could be completely deprecated, with the new automatic configuration
// for sticky headers. The "set" might still have a reason to exists, under evaluation.
getStickySectionHeadersHolder(); to getStickyHeaderContainer();

// I decided to not refactor this method anymore: The boolean parameter might
// become useful to set value from user configuration at run time.
// Normally, we should not call this method to hide headers startup by setting
// false, because they already hidden by default!
// I will just remove the dependency from the "animation on scrolling".
setDisplayHeadersAtStartUp(boolean displayHeaders);

// Let's remove these 2 wrappers, easier to manage:
// usually we have a boolean to check, after the refactoring we can avoid
// the if statement and set directly this boolean + custom sticky layout container
enableStickyHeaders(); to setStickyHeaders(true); + setStickyHeaders(true, ViewGroup); 
disableStickyHeaders(); to setStickyHeaders(false);
  • AnimatorAdapter.java
// Name is more coherent with the others
boolean isAnimationOnReverseScrolling(); to isAnimationOnReverseScrollingEnabled();

// Can't be supported anymore due to the new internal condition of non-animations
setAnimationStartPosition(int start);

// Use of parameter Holder instead of ItemView is more convenient
// Anyway, this method is automatically called by FlexibleAdapter only in case
// automap is active.
final void animateView(final View itemView, int position);
// to
final void animateView(final ViewHolder holder, int position);
  • SelectableAdapter.java
// It's an utility method, to move in Utils.java
static int getSpanCount(RecyclerView.LayoutManager layoutManager);

// The call is handled internally
public void resetActionModeFlags(); to private
  • Utils.java
// Deprecate defColor to color? We change the meaning of the parameter.
// And so, also Context becomes deprecated. I add also an overloaded method without
// color, but you must have the accentColor configured. Practical usages will be:
// from 
highlightText(@NonNull Context context, @NonNull TextView textView,
              String originalText, String constraint, @ColorInt int defColor);
// to
int color = fetchAccentColor(Context context, @ColorInt int defColor);
highlightText(@NonNull TextView textView,
              String originalText, String constraint, @ColorInt int color);
// to 2nd overloaded method with the default accentColor
highlightText(@NonNull TextView textView, String originalText, String constraint);

// Use of LayoutManager as parameter is more convenient
int getOrientation(RecyclerView recyclerView);
// to
int getOrientation(RecyclerView.LayoutManager)
  • DrawableUtils.java
// Just a better name
setBackground(View view, Drawable drawable);
setBackground(View view, @DrawableRes int drawableRes);
// to
setBackgroundCompat(View view, Drawable drawable);
setBackgroundCompat(View view, @DrawableRes int drawableRes);

// Changed name and return value
int getSelectableBackground(Context context);
// to
Drawable getSelectableItemBackground(Context context);

// Changed return value from (int) resourceId to (int) color
int getColorControlHighlight(Context context);

// RippleColor becomes the 3rd parameter
Drawable getSelectableBackgroundCompat(@ColorInt int normalColor,
					@ColorInt int pressedColor,
					@ColorInt int rippleColor)

// It's now public
ColorDrawable getColorDrawable(@ColorInt int color);

Deprecation

  • FlexibleAdapter.java
// I don't see any useful application 
int getItemCountOfTypesUntil(@IntRange(from = 0) int position, Integer... viewTypes);

// Orphan headers methods series: I don't see any advantages to keep in the adapter,
// most of the time the user has the control on the items to remove.
boolean isRemoveOrphanHeaders();
FlexibleAdapter setRemoveOrphanHeaders(boolean removeOrphanHeaders);
List<IHeader> getOrphanHeaders();

// Private methods (linked to OrphanHeaders methods)
void addToOrphanListIfNeeded(IHeader header, int positionStart, int itemCount);
void removeFromOrphanList(IHeader header);
boolean isHeaderShared(IHeader header, int positionStart, int itemCount);

// We simply can use item.setHeader(header) or item.setHeader(null).
// Also, it's not obvious that the 2 methods also show or hide header,
// we should use add/remove items.
FlexibleAdapter linkHeaderTo(@NonNull T item, @NonNull IHeader header);
IHeader unlinkHeaderFrom(@NonNull T item);

// The library doesn't use the concept of "Section index": we don't add sections
// using "Section index"
int getSectionIndex(@NonNull IHeader header);
int getSectionIndex(@IntRange(from = 0) int position);

// This is like a command to expand an expandable, it is unused
int addAllSubItemsFrom(...)
  • Support for DiffUtil
// DiffUtil is slower than the internal implementation, it will be released in rc1 with
// deprecated status, to be completely removed in the final version
boolean isAnimateChangesWithDiffUtil();
FlexibleAdapter setAnimateChangesWithDiffUtil(boolean useDiffUtil);
FlexibleAdapter setDiffUtilCallback(DiffUtilCallback diffUtilCallback);
synchronized void animateDiff(@Nullable List<T> newItems, Payload payloadChange)
public static class DiffUtilCallback<T> extends DiffUtil.Callback
  • FlexibleViewHolder.java
/*-------------------*/
/* TOUCHABLE METHODS */
/*-------------------*/
// Because Headers and Footers cannot be draggable, we can't deprecate anymore the touchable
// methods in IFlexible, instead, we forbid to override the 2 methods in FlexibleViewHolder
// that will be final.
// To disallow the touch of a specific item at runtime, users should change the flags in
// IFlexible only, those will be taken into account in ItemTouchHelperCallback and ViewHolder.
public boolean isDraggable(); will be final
public boolean isSwipeable(); will be final
  • OnUpdateListener
// The "size" now represents ONLY the main items:
// Scrollable Headers and Footers are not counted.
onUpdateEmptyView(int size);
  • EndlessScrollListener
// New callbacks
noMoreLoad(int newItemsSize);
onLoadMore(int lastPosition, int currentPage);

// Already removed!
onLoadMore();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant