Clever Geek Handbook

Setting a variable via a protected function - loss of reference

Update 2017 (in progress ...)

In general, the situation looks like this. I am pushing an instance B

(android snippet) and expect that onContextItemSelected

instance to be called. onContextItemSelected

called really, but it turns out that this method is an instance of the class C

.

I was asked to show the project. After looking at this code after about 2 years, I figured I should do a little clarification first, as this is not the event commented above, maybe 5%. I don't think I can do much more today. I've drawn some overview of the whole project so you know what to look at. It is not fully completed and may contain some errors, but more or less it looks like this:

enter image description here

I'll try to post clear steps to reproduce it tomorrow. But if you want to look at it, I'll include the files here anyway

A

BaseCustomFragment

B

and C

are classes that extend it.

Old

I have something like this:

public class A extends Fragment implements OnItemClickListener, OnItemLongClickListener{
    protected D loc;

    protected void setContext(D l){
        Log.d("A", "setContext :" + String.valueOf(l));
        loc = l;
        Log.d("A", "setContext2 :" + String.valueOf(loc));
    }

    public boolean onContextItemSelected(MenuItem item) {
        Log.d("A", "itemSelected :" + String.valueOf(loc));
    }
}

and

public class B extends A{

    public boolean onItemLongClick(AdapterView<?> pr, View view,int p, long id) {
        D d = (D) pr.getItemAtPosition(p);
        Log.d("B", "longClick :" + String.valueOf(d));
        setContext(d);
        return false;
    }
}

and the log looks like this:

B longClick: data

SetContext: data p>

SetContext2: data

A itemSelected: null

I don't touch loc

or setContex

anywhere else. I have absolutely no idea what's going on. How is this possible?

I am setting the A

class as a ListView

listener. I am using this snippet in ViewPager

. onItemSelected

called immediately after setContext

. I don’t know what to say more about this.

Edit:

To declare volatile didn't fix it, but ... Even stranger stuff - in the class A

I have:

public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
    lv = (ListView) inflater.inflate(layoutResourceId, container, false);
    lv.setOnItemClickListener(this);
    lv.setOnItemLongClickListener(this);
    lv.setAdapter(adapter);
    //this.registerForContextMenu(lv);
    return lv;
}

I also added to the class B

:

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
    super.onCreateView(inflater, container, savedInstanceState);
    this.registerForContextMenu(lv);
    return lv;
}
@Override
public boolean onContextItemSelected(MenuItem item) {
    //Never called!
    Log.d("Loc", "selected :" + name);
    return super.onContextItemSelected(item);
}

When the magic happens, I also have a class C

, which is basically the same B

only that I did not override these additional methods above:

public class C extends A {      
    @Override
    public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
        ...
    }   
    @Override
    public boolean onItemLongClick(AdapterView<?> parent, View view,int position, long id) {
        ...
    }       
}

And the log shows:

B longClick: data is from instance B

SetContext: data - from instance B

SetContext2: data - from instance B

A itemSelected: null - from instance C

!!!

How about this? I registerForContext

only call in class B

Any ideas?

+3


source to share


2 answers


Refresh

What's going wrong?

You are using ViewPager

in combination with FragmentPagerAdapter

. What an important implementation of this combination is that you have multiple Fragment

attached to Activity

. It is necessary to ViewPager

work as needed, at least the next and the previous one Fragment

with theirs View

to be ready to exchange.

What's wrong is that you don't take this behavior ViewPager

into account when posting MenuItem

using header only. This is generally a terrible idea, and these are just examples of why it is wrong. When the context menu is displayed and the user selects an item, it first MainActivity

has the ability to handle it, then the event goes to FragmentManager

, which dispatches the event to everything in the attached Fragment

(because there is no other sane choice if you think about it). So each of yours Fragment

gets a call onMenuItemSelected

and tries to handle it as the headers are the MenuItem

same. And of course you have an exception because there is no Fragment

currently selected item set for another "context".

How to fix it?

Obviously, don't just use a title to dispatch the event for MenuItem

s. In fact, you can generate unique groupId

or itemId

for each Fragment

. But I prefer more OOP. First, get rid of all your source code control context menus ( menus

, mpos

, setMenu

, addMenuItem

, removeItemMenu

, onMenuItemSelected

, etc.) and replace them with something like this

public class BaseCustomFragment extends Fragment implements OnItemClickListener, OnItemLongClickListener
{

    private List<MenuItemAction> menus = new ArrayList<MenuItemAction>();

    // base class for all actions for context-menu in BaseCustomFragment 
    abstract class MenuItemAction implements MenuItem.OnMenuItemClickListener
    {
        private final String title;

        public MenuItemAction(String title)
        {
            this.title = title;
        }

        public String getTitle()
        {
            return title;
        }

        public final boolean isVisible()
        {
            return isVisibleImpl(conCon, locCon);
        }

        @Override
        public final boolean onMenuItemClick(MenuItem item)
        {
            AdapterContextMenuInfo info = (AdapterContextMenuInfo) item.getMenuInfo();
            Log.d("MenuItem", "menu item '" + title + "' for #" + info.position + " of  " + BaseCustomFragment.this.getClass().getSimpleName());
            handleActionImpl(conCon, locCon);
            return true;
        }

        protected final void startActivity(Uri uri)
        {
            startActivity(Intent.ACTION_VIEW, uri);
        }

        protected final void startActivity(String action, Uri uri)
        {
            startActivity(new Intent(action, uri));
        }

        protected final void startActivity(Class<? extends Activity> activityClass)
        {
            startActivity(new Intent(getActivity(), activityClass));
        }

        protected final void startActivity(Intent intent)
        {
            getActivity().startActivity(intent);
        }

        protected abstract boolean isVisibleImpl(Customer conCon, Localization locCon);

        protected abstract void handleActionImpl(Customer conCon, Localization locCon);

    }

    protected void setContext(Customer c, Localization l)
    {
        Log.d("BaseFrag", "setContext class:" + String.valueOf(this.getClass()));
        Log.d("BaseFrag", "setContext :" + String.valueOf(l));
        conCon = c;
        locCon = l;
        Log.d("BaseFrag", "setContext2 :" + String.valueOf(locCon));
        updateContextMenu();
    }

    protected void setMenus(MenuItemAction... menuItems)
    {
        this.menus = Arrays.asList(menuItems);
        updateContextMenu();
    }

    @Override
    public void onCreateContextMenu(ContextMenu menu, View v, ContextMenuInfo info)
    {
        super.onCreateContextMenu(menu, v, info);

        for (MenuItemAction menuItemAction : menus)
        {
            if (menuItemAction.isVisible())
            {
                MenuItem menuItem = menu.add(menuItemAction.getTitle());
                menuItem.setOnMenuItemClickListener(menuItemAction);
            }
        }
    }


    private void updateContextMenu()
    {
        if (lv == null)
            return;
        boolean hasVisibleItems = false;
        for (MenuItemAction menuItemAction : menus)
        {
            if (menuItemAction.isVisible())
            {
                hasVisibleItems = true;
                break;
            }
        }
        if (hasVisibleItems)
        {
            Log.d(getClass().getSimpleName(), "Attaching context menu for " + getClass().getSimpleName());
            this.registerForContextMenu(lv);
        }
        else
        {
            Log.d(getClass().getSimpleName(), "Detaching context menu for " + getClass().getSimpleName());
            this.unregisterForContextMenu(lv);
        }
    }

Here MenuItemAction

is the base class for all "internal" menu items that will be stored Fragment

independently by everyone . I call them "internal" because the "real" MenuItem

are implementation details and you cannot inherit from them, so we will create "real" from our "internal" ones. This transformation / creation is done internally onCreateContextMenu

. Note that it MenuItemAction

dispatches events that already provide your "context" data where needed. It also contains several helper methods startActivity

to simplify your code.

Now we can add a BaseCustomFragment

bunch of helper methods to create typical menu items:

    protected MenuItemAction createNavigateToLocationMenuItem()
    {
        return new MenuItemAction("Navigate to location")
        {
            @Override
            protected boolean isVisibleImpl(Customer conCon, Localization locCon)
            {
                return (locCon != null);
            }

            @Override
            protected void handleActionImpl(Customer conCon, Localization locCon)
            {
                startActivity(Uri.parse("google.navigation:q=" + Uri.encode(locCon.noZipString())));
            }
        };
    }

    protected MenuItemAction createCallMenuItem()
    {
        return new MenuItemAction("Call")
        {
            @Override
            protected boolean isVisibleImpl(Customer conCon, Localization locCon)
            {
                return (conCon != null) && (conCon.getPhoneNumbers().size() > 0);
            }

            @Override
            protected void handleActionImpl(Customer conCon, Localization locCon)
            {
                startActivity(Intent.ACTION_DIAL, Uri.parse("tel:" + Uri.encode(BaseCustomFragment.this.conCon.getPhoneNumbers().get(0).toString())));
            }
        };
    }

    protected MenuItemAction createSmsMenuItem()
    {
        return new MenuItemAction("SMS")
        {
            @Override
            protected boolean isVisibleImpl(Customer conCon, Localization locCon)
            {
                return (conCon != null) && (conCon.getPhoneNumbers().size() > 0);
            }

            @Override
            protected void handleActionImpl(Customer conCon, Localization locCon)
            {
                startActivity(Uri.parse("sms:" + Uri.encode(BaseCustomFragment.this.conCon.getPhoneNumbers().get(0).toString())));
            }
        };
    }

    protected MenuItemAction createEmailMenuItem()
    {
        return new MenuItemAction("Email")
        {
            @Override
            protected boolean isVisibleImpl(Customer conCon, Localization locCon)
            {
                return (conCon != null) && (conCon.getEmail().length() > 0);
            }

            @Override
            protected void handleActionImpl(Customer conCon, Localization locCon)
            {
                startActivity(Uri.parse("mailto:" + Uri.encode(conCon.getEmail())));
            }
        };
    }

    protected MenuItemAction createNewOrderMenuItem()
    {
        return new MenuItemAction("New Order")
        {
            @Override
            protected boolean isVisibleImpl(Customer conCon, Localization locCon)
            {
                return true;
            }

            @Override
            protected void handleActionImpl(Customer conCon, Localization locCon)
            {
                startActivity(OrderActivity.class);
            }
        };
    }

So what is left to use these infrastructure-specific snippets like

public class CustomersFrag extends BaseCustomFragment
{
    public CustomersFrag()
    {

        ... 

        setMenus(createCallMenuItem(),
                 createSmsMenuItem(),
                 createEmailMenuItem(),
                 createNewOrderMenuItem(),
                 createNavigateToLocationMenuItem());
    }

    ... 

    @Override
    public boolean onItemLongClick(AdapterView<?> parent, View view, int position, long id)
    {
        Customer c = (Customer) parent.getItemAtPosition(position);
        setContext(c, c.getLocalization());
        return super.onItemLongClick(parent, view, position, id);
    }

or

public class LocalizationsFrag extends BaseCustomFragment
{
    public LocalizationsFrag()
    {

        ... 

        setMenus(
                createNavigateToLocationMenuItem(),
                createNewOrderMenuItem());
    }

    ... 

    @Override
    public boolean onItemLongClick(AdapterView<?> parent, View view, int position, long id)
    {
        Localization o = (Localization) parent.getItemAtPosition(position);
        setContext(null, o);
        return super.onItemLongClick(parent, view, position, id);
    }


Note that you no longer have if

/ else

or switch

by title of the menu item in your code. Also, you don't need to do your weird addMenuItem

/ removeMenuItem

in each onItemLongClick

depending on the data in certain chunks, because:

  • Everyone MenuItemAction

    now has their own methods isVisibleImpl

    andhandleActionImpl

  • setContext

    causes updateContextMenu

All of the menu actions for you seem to be generic and fit into the simple "context" you already have. So I made a MenuItemAction

simple non-static inner class that can get its parent Fragment

using BaseCustomFragment.this

. If at some point this becomes a limitation and you would like to create an action that belongs to the subclass BaseCustomFragment

and you want it to use the data from that snippet, you just need to create a menu item in the appropriate subclass. Imagine that you want to add a Copy Order menu item. You do something like this:

public class OrdersFragment extends BaseCustomFragment
{

    ... 


    private Order currentOrder;

    public boolean onItemLongClick(AdapterView<?> parent, View view, int position, long id)
    {
        Order o = (Order) parent.getItemAtPosition(position);
        currentOrder = o;
        setContext(o.getCustomer(), o.getLocalization());

        return super.onItemLongClick(parent, view, position, id);
    }

    protected MenuItemAction createNewOrderMenuItem()
    {
        return new MenuItemAction("Copy Order")
        {
            @Override
            protected boolean isVisibleImpl(Customer conCon, Localization locCon)
            {
                return (OrdersFragment.this.currentOrder != null);
            }

            @Override
            protected void handleActionImpl(Customer conCon, Localization locCon)
            {
                Intent intent = new Intent(getActivity(), OrderActivity.class);
                intent.putExtra("copySrcId", OrdersFragment.this.currentOrder.getId());
                startActivity(intent);
            }
        };
    }

and now your "Copy Order" MenuItemAction

has access to currentOrder

that defined inside OrdersFragment

.

Side Notes

Looking at your code, I noticed a few more things that I think are worth mentioning:

  • Using an integer type for a phone number is a bad idea. Phone numbers can contain additional characters such as "+" or "*" or "#" that will not fit into an integer type. Phone numbers can start with "0" and you will lose the integer conversion.
  • I think you better study the package java.util

    . For example, there is a method contains

    in java.util.List

    and its subclasses.
  • It is not clear why you are using very specific types in your declarations when you can use a custom superclass / interface. For example, all your Model classes seem to be using java.sql.Timestamp

    date fields for no apparent reason (do you really need nanoseconds?). Or you use it ArrayList

    everywhere, although sometimes just List

    enough
  • In MainActivity

    in, prepareTabs

    you don't call adapter.notifyDataSetChanged();

    after adding tabs. This actually crashes with the new support library.

Hope it helps

Old answer (and most likely irrelevant)

Add this

to every call Log

. I suspect the answer is that in

B longClick: data

SetContext: data p>

SetContext2: data

A itemSelected: null

somehow the latter Log

is performed for another object, which is the first 3. Is it also true that the problem is not reproducible every time? If possible, it is due to device rotation or something else triggering activity re-creation in the middle of event handling. Thus, you can add a journal in his onPause

, and onResume

see if this is true.

+1


source


If your object is A

accessing multiple threads (which is most likely given to your event listeners), you might see problems like this (the common terminology for it is "stale data" ). To quickly check if this is a streaming issue, declare a member variable like protected volatile D loc;

and see if the issue goes away (see this resource about volatile ). If this does solve the problem, you will need to implement better streaming protections to make sure you don't get confused by more nefarious / subtle streaming bugs.



+1


source






More articles:


All Articles