Custom Callback Mechanism vs Standard Implementation in Android

androidcallbackscoding-styleobject-oriented

I am trying to create a simple app which has 2 activities, which enter text on one activity and then displays it on the title bar if another activity:

EditTextActivity:

enter image description here

MainActivity:

enter image description here

I know the standard way to implement it is to use startActivityForResult:

public class EditTextActivity extends AppCompatActivity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_text_view);
    }

    public void buttonPressed(View view){
        Intent intent=new Intent();
        Intent.putExtra("result",    ((EditText)this.findViewById(R.id.editText)).getEditableText().toString());
        this.setResult(0,intent);
        finish();
    }
}

–

public class MainActivity extends AppCompatActivity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        this.startActivityForResult(new     Intent(this,EditTextActivity.class),0);
    }

    @Override
    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
        this.setTitle(data.getExtras().getString("result"));
    }
}

but I think this mechanism is not convenient: if I have multiple startActivityForResult, my onActivityResult would be very long. Also it has poor readability because the logic to handle the result is inside onActivityResult, which is far away from startActivityForResult.

Currently android can't put Objects (callback) into intent, so I try to implement my own callback passing mechanism: which opens a static map to store callback from other activity:

public class EditTextActivity extends AppCompatActivity {
    public static HashMap<Integer,Callback> callbackMap=new HashMap<>();

    public static void startActivity(AppCompatActivity activity,Callback callback){
    //add callback
    int activityObjectId=System.identityHashCode(activity);
    callbackMap.put(activityObjectId,callback);

    Intent intent=new Intent(activity,EditTextActivity.class);
    intent.putExtra("activityObjectId",System.identityHashCode(activity));
    activity.startActivity(intent);
}

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

    public void buttonPressed(View view){
        int activityObjectId=this.getIntent().getIntExtra("activityObjectId",0);
        //remove callback      callbackMap.get(activityObjectId).onActivityResult(((EditText)this.findViewById(R.id.editText)).getEditableText().toString());
        callbackMap.remove(activityObjectId);
        finish();
    }

    public interface Callback{
        void onActivityResult(String s);
    }
}

then wrap the code onActivityResult into an object and then put it into the map before startActivity:

public class MainActivity extends AppCompatActivity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        EditTextFragment.startActivity(this,R.id.rootView, new EditTextFragment.Callback() {
            @Override
            public void onActivityResult(String s) {
                MainActivity.this.setTitle(s);
            }
        });
    }
}

I'm going to implement this callback mechanism (if possible) in activity, fragments, and other components that needs callback handler afterwards as my standard way, because I think it allows me to use an unique way to implement callbacks, is it a good idea to do so?

Best Answer

Static globals should be avoided whenever possible. They introduce many more problems than they solve, and there are usually design changes that can eliminate the need for global state.

I think [the framework provided] mechanism is not convenient: if I have multiple startActivityForResult, my onActivityResult would be very long. Also it has poor readability because the logic to handle the result is inside onActivityResult, which is far away from startActivityForResult.

I agree with your analysis. Let me suggest a different design that addresses your concerns without using global state.

interface ActivityResultListener {
  void onActivityResult(int resultCode, Intent data);
} 

public class CallbackResultActivity extends AppCompatActivity {
  private final Map<Integer, ActivityResultListener> callbacks = new HashMap<>();
  private final AtomicInteger requestCode = new AtomicInteger(0);

  protected startActivityForResult(
    Intent intent, 
    ActivityResultListener callback
  ) {
    int newRequestCode = requestCode.incrementAndGet();
    callbacks.put(newRequestCode, callback);
    startActivityForResult(intent, newRequestCode);
  }

  protected final onActivityResult(int requestCode, int resultCode, Intent data) {
    ActivityResultListener callback = callbacks.get(requestCode);
    if (callback != null) {
      callback.onActivityResult(resultCode, data);
      callbacks.remove(requestCode);
    }
  }
}

This creates an extension on the base AppCompatActivity that hides the notion of requestCodes and allows you to call startActivityForResult and pass a callback. It's pretty similar to your implementation, but it's generic, reusable, testable, and EditTextActivity doesn't have to know anything about callbacks. You can use it like so:

public class MainActivity extends CallbackResultActivity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        this.startActivityForResult(new Intent(this, EditTextActivity.class), 
          new ActivityResultListener() {
            @Override
            public void onActivityResult(int resultCode, Intent data) {
                this.setTitle(data.getExtras().getString("result"));
            }
          });
    }
}