My solution to Chalenge 1 . your opinion?

hi friends
i wanted to show my solution that works good but maybe there is something to notice so i will appreciate to any creative advice . is it good code or bad ?
i used interface to communicate between fragments
i deleted unnecessary codes and summarized it

CrimeListFragment.Java

private static final int REQUEST_CRIME = 0;
 ...
private abstract class GenericCrimeHolder extends RecyclerView.ViewHolder implements View.OnClickListener {
 ...
    public void onClick(View view) {
        Intent intent = CrimeActivity.newIntent(getActivity(),mCrime.getId(),getLayoutPosition());
        startActivityForResult(intent,REQUEST_CRIME);
    }
}

...
@Override
public void onActivityResult(int requestCode, int resultCode, Intent data) {
    super.onActivityResult(requestCode, resultCode, data);
    if(resultCode != Activity.RESULT_OK) {
        return;
    }
    if(requestCode == REQUEST_CRIME) {
        if(data == null) {
            return;
        }
        int layoutPosition = CrimeActivity.resultDecodeToLayoutIdInt(data);
        mAdapter.notifyItemChanged(layoutPosition);
    }
}

}


CrimeActivity.java

public class CrimeActivity extends SingleFragmentActivity implements CrimeFragment.ReAction {
private static final String EXTRA_CRIME_ID = “com.bignerdranch.android.criminalintent.crime_id”;
private static final String EXTRA_CRIME_LAYOUT_ID = “com.bignerdranch.android.criminalintent.crime_layout_id”;
private static final String EXTRA_CRIME_LAYOUT_ID_RESULT = “com.bignerdranch.android.criminalintent.crime_layout_id_result”;

public static Intent newIntent(Context packageContext, UUID crimeID ,int layoutId) {
    Intent intent = new Intent(packageContext , CrimeActivity.class);
    intent.putExtra(EXTRA_CRIME_ID,crimeID);
    intent.putExtra(EXTRA_CRIME_LAYOUT_ID,layoutId);
    return intent;
}
public static int resultDecodeToLayoutIdInt(Intent intent) {
    int layoutId = intent.getIntExtra(EXTRA_CRIME_LAYOUT_ID_RESULT,0);
    return layoutId;
}
@Override
protected Fragment createFragment() {
    UUID crimeId = (UUID) getIntent().getSerializableExtra(EXTRA_CRIME_ID);
    return CrimeFragment.newInstance(crimeId);
}
@Override
public void isChanged() {
    Intent data = new Intent();
    int layoutId = getIntent().getIntExtra(EXTRA_CRIME_LAYOUT_ID,0);
    data.putExtra(EXTRA_CRIME_LAYOUT_ID_RESULT,layoutId);
    setResult(RESULT_OK,data);
}

}


CrimeFragment.java

public class CrimeFragment extends Fragment {

private ReAction mCallback;
private static final String ARG_CRIME_ID = "crime_id";
...
public interface ReAction {
    public void isChanged();
}
@Override
public void onAttach(Context context) {
    super.onAttach(context);
    try {
        mCallback = (ReAction) context;
    }
    catch (ClassCastException e) {
        throw new ClassCastException(context.toString() + " must implement ReAction interface");
    }
}
@Nullable
@Override
public View onCreateView(LayoutInflater inflater,ViewGroup container,Bundle savedInstanceState) {
    //return super.onCreateView(inflater, container, savedInstanceState);
   View vd = inflater.inflate(R.layout.fragment_crime,container,false);
    mTitleField = (EditText) vd.findViewById(R.id.crime_title);
    mTitleField.setText(mCrime.getTitle());
    mTitleField.addTextChangedListener(new TextWatcher() {
        @Override
        public void beforeTextChanged(CharSequence s, int start, int count, int after) {
        }
        @Override
        public void onTextChanged(CharSequence s, int start, int before, int count) {
            mCrime.setTitle(s.toString());
            mCallback.isChanged();
        }
        @Override
        public void afterTextChanged(Editable s) {
        }
    });
    mDateButton =(Button) vd.findViewById(R.id.crime_date);
    mDateButton.setText(mCrime.getDate().toString());
    mDateButton.setEnabled(false);
    mSolvedCheckBox = (CheckBox) vd.findViewById(R.id.crime_solved);
   mSolvedCheckBox.setChecked(mCrime.isSolved());
    mSolvedCheckBox.setOnCheckedChangeListener(new OnCheckedChangeListener() {
        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
            mCrime.setSolved(isChecked);
            mCallback.isChanged();
        }
    });
    return vd;
}

}

1 Like

That seems like a lot of work!

A simpler way is have a few lines of code that covers most situations, then a couple of lines of fail-safe code that will do a full update if necessary.

Add a new variable in the model of CrimeListFragment.java:

    private int mLastUpdatedPosition = -1; //Challenge: Efficient RecyclerView Reloading

Then use the saved position if you have one (and you will even if you rotate the device), or do a full update otherwise:

    private void updateUI() {
        CrimeLab crimeLab = CrimeLab.get(getActivity());
        List<Crime> crimes = crimeLab.getCrimes();

        if (mAdapter == null) {
            mAdapter = new CrimeAdapter(crimes);
            mCrimeRecyclerView.setAdapter(mAdapter);
        } else {
            if (mLastUpdatedPosition > -1) {
                mAdapter.notifyItemChanged(mLastUpdatedPosition);
                mLastUpdatedPosition = -1;
            } else {
                mAdapter.notifyDataSetChanged();
            }
        }
    }

You will of course need to save the position on click:

        @Override
        public void onClick(View view) {
            Intent intent = CrimeActivity.newIntent(getActivity(), mCrime.getId());
            mLastUpdatedPosition = this.getAdapterPosition(); //Challenge: Efficient RecyclerView Reloading
            startActivity(intent);
        }
9 Likes

hi ,Qarj
could you explain that why" mLastUpdatedPosition > -1 "?
and what’s the meaning of this line?
I got really confused…
thanks a lot

Hi @wolfheros, that line is checking that mLastUpdatedPosition has a value that was set by the code in onClick.

If the value is 0 or higher, that means the user clicked on a row, and we still remember what row they clicked on.

If it is -1, then that means that they have not clicked on a row, or that perhaps due to low memory that information got discarded so we need to update all rows just in case.

The -1 is just a lazy way of representing a null value, since we the position number will always be a positive value.

Thanks for posting this answer. I just came to the site after solving the challenge since I wanted to check my answer. This is the same code I came up with also. (getAdapterPostion(), etc)
Very nice because it is such a small solution – just three lines of code or so.

Clearly simple, and looks very nice. I wouldn’t have gone with this because it’s not necessarily “correct” given an asynchronous model. I had assumed that the code being invoked in the onClick() and in updateUI() might be across threads and thus the member variable would need to be volatile or an AtomicInt or something…

But, I’m a total newb and I don’t know what the concurrency model of the Android framework is… can anyone enlighten me?

It seems like this all runs in the main thread - this is discussed quite a bit in chapter 25. Check out in particular pages 481 and 482.

So I think this is safe and correct, unless someone can explain specifically why it isn’t?

Thanks. I clearly haven’t gotten to that part yet. Nice to have the early
clarification.

The most simple solution I found is:

  1. Add new int field:

private int adapterPosition;

  1. Change onClick so you “catch” the position, when you click on list item:

public void onClick(View view) {
adapterPosition = getAdapterPosition();
Intent intent = CrimeActivity.newIntent(getActivity(), crime.getId());
startActivity(intent);
}

  1. Use notifyItemChanged and pass adapterPosition to it:
private void updateUI() {
    CrimeLab crimeLab = CrimeLab.get(getActivity());
    List<Crime> crimes = crimeLab.getCrimes();

    if (adapter == null) {
        adapter = new CrimeAdapter(crimes);
        crimeRecyclerView.setAdapter(adapter);
    } else {
        adapter.notifyItemChanged(adapterPosition);
    }

}

That’s all!

7 Likes

nice solution! but why do you add mAdapter.notifyDataSetChanged()? I think it works also without it

@theHalfBloodPrince My comment from 11 Aug 2017 explains why…

It works using getAdapterPosition() without ‘this’.

I have no opinion but I suggest you to go Best Sad Status to know best fix in this matter.

thank for sharing!! it work !!