Accessing class variables directly is dirty and needs to stop. Make all class level variables private and provide a better API to access them

Comment on attachment 8499634 [details] [diff] [review] Remove package level access to class variables within the tabs package Review of attachment 8499634 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but needs better encapsulation. ::: mobile/android/base/tabs/TabsGridLayout.java @@ +55,4 @@ > @Override > public void onMovedToScrapHeap(View view) { > TabsLayoutItemView item = (TabsLayoutItemView) view; > + item.getThumbnail().setImageDrawable(null); Just do setThumbnail(Drawable) instead of exposing individual views. @@ +72,4 @@ > @Override > public void onClick(View v) { > TabsLayoutItemView itemView = (TabsLayoutItemView) v.getTag(); > + Tab tab = Tabs.getInstance().getTab(itemView.getId()); getId -> getTabId() for clarity. ::: mobile/android/base/tabs/TabsLayoutItemView.java @@ +110,5 @@ > + public int getId() { > + return mId; > + } > + > + public TextView getTitle() { No need to add getters for stuff that is not being used. Besides, we probably want to hide details of the internal view structure. So, remove all these unused view getters. ::: mobile/android/base/tabs/TabsListLayout.java @@ +80,5 @@ > @Override > public void onMovedToScrapHeap(View view) { > TabsLayoutItemView item = (TabsLayoutItemView) view; > + item.getThumbnail().setImageDrawable(null); > + item.getCloseButton().setVisibility(View.VISIBLE); Add a setCloseVisible(boolean) instead.

Ack - that was sloppy of me. Encapsulation sorted now. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=dbe615cff1fd

Comment on attachment 8500446 [details] [diff] [review] Remove package level access to class variables within the tabs package Review of attachment 8500446 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thanks.