Review request of dual-delete challenge solution

Android Programming (3rd Edition) 17. Two-Pane Master-Detail Interfaces
1

I added the Ch 13 & Ch 17 delete solutions at once. I think I’ve found a complete minimal solution, but I’m not sure. Goals:

  • delete the crime from the database
  • update the list UI
  • if the removed crime is currently displayed in a CrimeFragment
    • finish() if we’re single pane
    • remove the fragment if we’re two pane

The problem was how to tell whether the deleted crime is the one being displayed in the two-pane CrimeFragment. I ended up creating a member variable, mCurrentCrime, in CrimeListFragment. Then, I updated the onCrimeSelected(Crime) to delete the CrimeFragment if passed null. I also added an onCrimeRemoved(Crime) callback to CrimeFragment so it can report back to CrimeListFragment.removeCrimeFromUI(Crime) or call finish() depending on which activity’s callback is called. The removeCrimeFromUI(Crime) method checks if the UUIDs match for crime and mCurrentCrime and calls onCrimeSelected(null) if necessary.

Is there a better way to do this? (more concise, etc.) I don’t like that I delete the crime from the database in both CrimeFragment and CrimeListFragment, but I guess that’s ok.

Here’s an abbreviated diff of the changes (some imports removed…):

@@ -52,6 +55,7 @@ public class CrimeFragment extends Fragment {
      */
     public interface Callbacks {
         void onCrimeUpdated(Crime crime);
+        void onCrimeRemoved(Crime crime);
     }
 
     private static final String ARG_CRIME_ID = "crime_id";
@@ -79,6 +83,7 @@ public class CrimeFragment extends Fragment {
     @Override
     public void onCreate(@Nullable Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
+        setHasOptionsMenu(true);
         UUID crimeId = (UUID) getArguments().getSerializable(ARG_CRIME_ID);
         mCrime = CrimeLab.get(getActivity()).getCrime(crimeId);
         mPhotoFile = CrimeLab.get(getActivity()).getPhotoFile(mCrime);
@@ -232,6 +237,24 @@ public class CrimeFragment extends Fragment {
         mCallbacks = null;
     }
 
+    @Override
+    public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
+        super.onCreateOptionsMenu(menu, inflater);
+        inflater.inflate(R.menu.fragment_crime, menu);
+    }
+
+    @Override
+    public boolean onOptionsItemSelected(MenuItem item) {
+        switch (item.getItemId()) {
+            case R.id.remove_crime:
+                CrimeLab.get(getActivity()).removeCrime(mCrime);
+                mCallbacks.onCrimeRemoved(mCrime);
+                return true;
+            default:
+                return super.onOptionsItemSelected(item);
+        }
+    }
+
     private void updateCrime() {
         CrimeLab.get(getActivity()).updateCrime(mCrime);
         mCallbacks.onCrimeUpdated(mCrime);
modified   CriminalIntent/app/src/main/java/com/bignerdranch/android/criminalintent/CrimeLab.java
@@ -35,6 +35,12 @@ public class CrimeLab {
         mDatabase.insert(CrimeTable.NAME, null, values);
     }
 
+    public void removeCrime(Crime crime) {
+        mDatabase.delete(CrimeTable.NAME,
+                CrimeTable.Cols.UUID + " = ?",
+                new String[]{crime.getId().toString()});
+    }
+
     public List<Crime> getCrimes() {
         List<Crime> crimes = new ArrayList<>();
         CrimeCursorWrapper cursor = queryCrimes();
modified   CriminalIntent/app/src/main/java/com/bignerdranch/android/criminalintent/CrimeListActivity.java
@@ -18,6 +18,15 @@ public class CrimeListActivity extends SingleFragmentActivity
 
     @Override
     public void onCrimeSelected(Crime crime) {
+        if (crime == null) {
+            CrimeFragment detailFragment = (CrimeFragment) getSupportFragmentManager()
+                    .findFragmentById(R.id.detail_fragment_container);
+            getSupportFragmentManager().beginTransaction()
+                    .remove(detailFragment)
+                    .commit();
+            return;
+        }
+
         if (findViewById(R.id.detail_fragment_container) == null) {
             Intent intent = CrimePagerActivity.newIntent(this, crime.getId());
             startActivity(intent);
@@ -30,10 +39,18 @@ public class CrimeListActivity extends SingleFragmentActivity
         }
     }
 
+    private CrimeListFragment getListFragment() {
+        return (CrimeListFragment) getSupportFragmentManager()
+                .findFragmentById(R.id.fragment_container);
+    }
+
     @Override
     public void onCrimeUpdated(Crime crime) {
-        CrimeListFragment listFragment = (CrimeListFragment) getSupportFragmentManager()
-                .findFragmentById(R.id.fragment_container);
-        listFragment.updateUI();
+        getListFragment().updateUI();
+    }
+
+    @Override
+    public void onCrimeRemoved(Crime crime) {
+        getListFragment().removeCrimeFromUI(crime);
     }
 }
modified   CriminalIntent/app/src/main/java/com/bignerdranch/android/criminalintent/CrimeListFragment.java
@@ -26,6 +26,7 @@ public class CrimeListFragment extends Fragment {
     private CrimeAdapter mCrimeAdapter;
     private boolean mSubtitleVisible;
     private Callbacks mCallbacks;
+    private Crime mCurrentCrime = null;
 
     /**
      * Required interface for hosting activities
@@ -62,6 +63,7 @@ public class CrimeListFragment extends Fragment {
 
         @Override
         public void onClick(View view) {
+            mCurrentCrime = mCrime;
             mCallbacks.onCrimeSelected(mCrime);
         }
     }
@@ -113,6 +115,22 @@ public class CrimeListFragment extends Fragment {
         View v = inflater.inflate(R.layout.fragment_crime_list, container, false);
         mCrimeRecyclerView = (RecyclerView) v.findViewById(R.id.crime_recycler_view);
         mCrimeRecyclerView.setLayoutManager(new LinearLayoutManager(getActivity()));
+        ItemTouchHelper itemTouchHelper = new ItemTouchHelper(
+                new ItemTouchHelper.SimpleCallback(0, ItemTouchHelper.LEFT) {
+                    @Override
+                    public boolean onMove(RecyclerView recyclerView, RecyclerView.ViewHolder viewHolder, RecyclerView.ViewHolder target) {
+                        return false;
+                    }
+
+                    @Override
+                    public void onSwiped(RecyclerView.ViewHolder viewHolder, int direction) {
+                        int position = viewHolder.getAdapterPosition();
+                        Crime crime = mCrimeAdapter.mCrimes.get(position);
+                        CrimeLab.get(getActivity()).removeCrime(crime);
+                        removeCrimeFromUI(crime);
+                    }
+                });
+        itemTouchHelper.attachToRecyclerView(mCrimeRecyclerView);
 
         if (savedInstanceState != null)
             mSubtitleVisible = savedInstanceState.getBoolean(SAVED_SUBTITLE_VISIBLE);
@@ -121,6 +139,15 @@ public class CrimeListFragment extends Fragment {
         return v;
     }
 
+    public void removeCrimeFromUI(Crime crime) {
+        if (mCurrentCrime != null
+                && mCurrentCrime.getId().equals(crime.getId())) {
+            mCallbacks.onCrimeSelected(null);
+            mCurrentCrime = null;
+        }
+        updateUI();
+    }
+
     public void updateUI() {
         CrimeLab crimeLab = CrimeLab.get(getActivity());
         List<Crime> crimes = crimeLab.getCrimes();
modified   CriminalIntent/app/src/main/java/com/bignerdranch/android/criminalintent/CrimePagerActivity.java
@@ -57,4 +57,9 @@ public class CrimePagerActivity extends AppCompatActivity
     @Override
     public void onCrimeUpdated(Crime crime) {
     }
+
+    @Override
+    public void onCrimeRemoved(Crime crime) {
+        finish();
+    }
 }
new file   CriminalIntent/app/src/main/res/drawable-hdpi/ic_menu_remove.png
new file   CriminalIntent/app/src/main/res/drawable-mdpi/ic_menu_remove.png
new file   CriminalIntent/app/src/main/res/drawable-xhdpi/ic_menu_remove.png
new file   CriminalIntent/app/src/main/res/drawable-xxhdpi/ic_menu_remove.png
new file   CriminalIntent/app/src/main/res/menu/fragment_crime.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<menu xmlns:android="http://schemas.android.com/apk/res/android"
+      xmlns:app="http://schemas.android.com/apk/res-auto">
+    <item
+        android:id="@+id/remove_crime"
+        android:icon="@drawable/ic_menu_remove"
+        android:title="@string/remove_crime"
+        app:showAsAction="ifRoom|withText"/>
+
+</menu>
\ No newline at end of file
modified   CriminalIntent/app/src/main/res/values/strings.xml
@@ -6,6 +6,7 @@
     <string name="crime_solved_label">Solved</string>
     <string name="date_picker_title">Date of crime:</string>
     <string name="new_crime">New Crime</string>
+    <string name="remove_crime">Remove Crime</string>
     <string name="show_subtitle">Show Subtitle</string>
     <string name="hide_subtitle">Hide Subtitle</string>
     <string name="subtitle_format">%1$d crimes</string>
2

I can see two possible bugs in your code:

  1. On a tablet, select a crime, rotate the screen, then swipe-delete the selected crime.
  2. On a phone, select a crime, press the back button, then swipe-delete the previously selected crime.

I would probably call back to the hosting activity from CrimeListFragment whenever you swipe a crime away. CrimeListActivity can then ask its CrimeFragment which Crime it has and act accordingly. This will put all of the delete logic in the callback onCrimeRemoved() method, and it also means that CrimeListFragment won’t have to assume as much about other fragments it might coexist with.