Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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();
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant