2
\$\begingroup\$

I have different ViewHolder for different Feed implementations. But I can't use the best practice of having each ViewHolder corresponds to different layout id because i am reusing layout for 2 different ViewHolder (aka. FeedWithImageHolder and FeedWithoutImageHolder)

class FeedAdapter extends RecyclerView.Adapter<FeedAdapter.BaseFeedHolder>{

    @NonNull
    @Override
    public BaseFeedHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
        View v = null;
        switch (viewType){
            case FeedType.FEED_WITH_IMAGE :
                v = LayoutInflater.from(context).inflate(R.layout.feed_status,parent,false);
                //notice that i use the same R.layout.feed_status for two different ViewHolder

                return new FeedWithImageHolder(v);
            case FeedType.FEED_WITHOUT_IMAGE:
                v = LayoutInflater.from(context).inflate(R.layout.feed_status,parent,false);
                return new FeedWithoutImageHolder(v);
            case FeedType.ADS:
                v = LayoutInflater.from(context).inflate(R.layout.ads,parent,false);
                return new AdsHolder(v);
        }
        return null;
    }

    @Override
    public void onBindViewHolder(@NonNull BaseFeedHolder holder, int position) {
        //noinspection unchecked
        holder.bindFeed(feeds.get(position));
    }

    abstract class BaseFeedHolder<T extends Feed> extends RecyclerView.ViewHolder {
        BaseFeedHolder(@NonNull View itemView) {
            super(itemView);
        }

        abstract void bindFeed(T feed);
    }

    class FeedWithImageHolder extends BaseFeedHolder<FeedStatus>{

        FeedWithImageHolder(@NonNull View itemView) {
            super(itemView);
        }

        @Override
        void bindFeed(FeedStatus feed) {
            doSomethingWithFeed(feed);
            bindImage(feed.getPostImage());
        }

        protected void bindImage(String image){
            clipImageToImageView(image);
        }
    }

    class FeedWithoutImageHolder extends FeedWithImageHolder{

        FeedWithoutImageHolder(@NonNull View itemView) {
            super(itemView);
            removeImageViewFromCoinstraintLayout(itemView);
        }

        @Override
        protected void bindImage(String image) {
            //void
        }
    }

    class AdsHolder extends BaseFeedHolder<Ads>{
        AdsHolder(@NonNull View itemView) {
            super(itemView);
        }

        @Override
        void bindFeed(Ads ads) {
        }
    }

}

Instead what I do is making Feed have a method .getType() so every implementation can have different type corresponding to it.

For example:

class FeedStatus implements Feed { 

    private String image;

    @Override
    public int getType() {
        return (this.image == null) ? FeedType.FEED_WITHOUT_IMAGE : FeedType.FEED_WITH_IMAGE;
    }

}

There is tingling worry here because I think there is code smell in there but I am not sure which one and I hope you can help me with it. Thanks

\$\endgroup\$

1 Answer 1

0
\$\begingroup\$

From a quick glance, I share your concerns.

One 'smell' is that a sub-class (FeedWithoutImageHolder) provides less functionality than the parent class, i.e. it does not actually "extend" the class.

I suggest trying to align layout structure and functionality:

Create two different layouts: The first one without an image, and a second one which includes the first and extends it by adding an image. Depending on your desired layout design, you may instead want to declare only one layout which includes some empty placeholder element of the desired image display size which either stays empty or receives an image, so that all visual elements align.

I recommend against having a Feed handle the type of layout at all. Keep data objects and layout concerns separate, that's what the Adapter is for. Have the adapter check if an image needs to be displayed for an item and determine the appropriate view type in Adapter.getItemViewType().

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for answering. So what you mean is i should handle the type decision on Adapter.getItemViewType() right ? i will do that now. Another thing, Should i make different layout for things which differ in only small way ? isn't it costly memory-wise ? \$\endgroup\$ May 24, 2019 at 13:18
  • \$\begingroup\$ Yes, Adapter.getItemViewType() should be used to determine and return which type of view should be used for an item. This also allows the RecyclerView to do its thing in recycling existing views of a given view type to display another item of the same view type. - I don't think that layout definitions will consume any significant amount of memory; more important to me would be to build layouts out of reusable sub-layouts, to reduce redundancies, facilitate future changes and help keeping the overall app layout consistent. \$\endgroup\$
    – JimmyB
    May 27, 2019 at 9:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.