Best practices for start() and stop() in Android MVP?

Question

I have some questions regarding the Presenter’s start(), stop() method. What would you normally put into these methods to prevent memory leaks or any potential problem.

For example, I have an Activity that host a VideoView. The videoPath passed to the Activity will be passed to the Presenter to a VideoUtility to trim the original video into a shorter one before getting passed back to the Activity to be played with the VideoView.

Here’s the confusion: I don’t know where is the appropriate place to call the trimVideo() method as it essentially only need to happen once (unlike in the Android Architect Blueprint, the task is updated with latest data, and thus it’s put in the onResume()).

Please see the code snippet below:

VideoEditorContract:

public interface VideoEditorContract {
  interface View extends BaseView<Presenter> {
      void playTrimVideo(String trimmedVideoPath);
  }

  interface Presenter extends BasePresenter {
  }
}

VideoEditorActivityBase:

public class VideoEditorActivityBase extends AppCompatActivity implements VideoEditorContract.View {
  private VideoEditorContract.Presenter mPresenter;

  @Override
  protected void onCreate(@Nullable Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_video_editor);

    String videoPath = getIntent().getStringExtra(RequestCode.EXTRA_VIDEO_PATH);

    mPresenter = new VideoEditorPresenter(videoPath, this);
  }

  @Override
  public void onResume(){
    super.onResume();
    mPresenter.start();
  }

  @Override
  public void playTrimVideo(String trimmedVideoPath) {
    final VideoView vv = findViewById(R.id.act_video_editor_videoView);
    vv.setVideoPath(trimmedVideoPath);
    vv.setOnPreparedListener(new MediaPlayer.OnPreparedListener() {
        @Override
        public void onPrepared(MediaPlayer mp) {
            vv.start();
        }
    });
  }

  @Override
  public void setPresenter(VideoEditorContract.Presenter presenter) {
    //do nothing as this activity has already init a presenter
  }
}

VideoEditorPresenter:

public class VideoEditorPresenter implements VideoEditorContract.Presenter {
  private final VideoEditorContract.View mVideoEditorView;

  @NonNull
  private String mVideoPath;

  public VideoEditorPresenter(@NonNull String videoPath, @NonNull VideoEditorContract.View videoEditorView) {
    mVideoPath = checkNotNull(videoPath);
    mVideoEditorView = checkNotNull(videoEditorView, "videoEditorView cannot be null!");
    mVideoEditorView.setPresenter(this);
    //trimVideo(); //should I do it here since this task is only need to be done once
  }

  @Override
  public void start() {
    //trimVideo(); //I can do it here but then I need to introduce a control variable; not sure if this is the best practice
  }

  private void trimVideo() {
    //trim video stuff
  }
   // Currently it doesn't have a stop() method. But if it have one,
   // what should I put in it? Releasing and clean up the 
   // VideoUtility I suppose?
}

score 0 · Answer 1 · edited Jun 20 '20 at 09:12

I got the answer from Francesco Cervone in Medium about this matter (his article is also an excellent resource on MVP, btw. Very well in tune with the Android Architect Blueprint). I leave the relevant bit here for future reader.

Hi, thank you.

Well, I think that the video should be trimmed in the Presenter#start(). Then, after the video has been trimmed, the presenter should call view.playTrimmedVideo(). You shouldn’t do anything in the presenter constructor.

I suppose the video “editing” is something expensive, so you should do that in a separate thread (using for example an async task). You need to implement the Presenter#stop() method because you have to cancel ongoing operations if there are any, unless you retain the presenter.

You said that the trimVideo should be called just once. You could cache/persist in some way the result of trimVideo so that if the video has been already trimmed, you use it.

I hope I answered your question.

"Could you elaborate more on why shouldn’t we put anything in the Presenter’s constructor? I’ve seen the Presenter’s bare minimal constructor in a couple of places but I don’t understand the reason behind it."

First, it’s a responsibility problem: you are going to create an instance of Presenter, and I don’t think that the video editing is something that belongs to the construction of that object.

Second, you don’t know when the presenter is being instantiated, so you shouldn’t execute expensive tasks in the constructor. If you use some dependency injection framework, the construction of the Presenter would be managed by the framework itself and it needs to be efficient. The construction of other objects could depend on the presenter one.

Best practices for start() and stop() in Android MVP?

1 Answers1