UdhayaBalaji / androidannotations

Automatically exported from code.google.com/p/androidannotations
0 stars 0 forks source link

Manage extras on the static helpers that help start an Activity #155

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I'm refetring to issue 143 "Provide static helpers to start activities".

I'm starting to this simple fact : we already have the @Extra annotation, so we 
know the type and name of the extras this activity will use.

I would great to be able to do something strongly-typed like :

// ExampleActivity
@Extra("my_string_extra")
String myMessage;

@Extra("my_date_extra")
Date myDateExtraWithDefaultValue = new Date();

// In another activity
ExampleActivity_.build(this)
.myMessage("aString")
.myDateExtraWithDefaultValue(new Date())
.start();

In addition to this, we could add some "required" feature in @Extra annotation. 
See the following :

@Extra("my_string_extra", required=true)
String myMessage;

@Extra("my_date_extra")
Date myDateExtraWithDefaultValue = new Date();

// With required extra
ExampleActivity_.build(this)
.myDateExtraWithDefaultValue(new Date())
.start("aString");

// With flags and requestCode
ExampleActivity_.build(this)
.myDateExtraWithDefaultValue(new Date())
.flags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP)
.startForResult(MY_REQUEST_CODE, "aString");

Also, I don't think "build" is a good name, I was referring to the builder 
pattern, but I can't think of something appropriate, maybe "prepare", or 
"from". I like "from", as long as we only put the context as the argument.

Original issue reported on code.google.com by jzap...@excilys.com on 30 Dec 2011 at 9:07

GoogleCodeExporter commented 8 years ago
That's a very good idea!

Whether we should use a builder pattern or not is also interesting. I like 
builder patterns (the prefs has them..), but I also realize they introduce 
quite some complexity when you don't know them. Indeed, they are good for 
"optional parameters", and we'll have a lot of optional params here (extras, 
flags).

I'm starting to think that maybe I should removed the overloaded methods 
created in issue 143, and replace them with this kind of builder.

Maybe we could name it "intent" instead of "build", and have a "build" and a 
"start" method on the builder.

Here is the idea :

Intent intent = ExampleActivity_.intent(this)
 .myDateExtraWithDefaultValue(new Date())
 .flags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP)
 .build();

ExampleActivity_.intent(this)
 .myDateExtraWithDefaultValue(new Date())
 .flags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP)
 .startForResult(MY_REQUEST_CODE);

Also note that there would be two versions of the "intent" method, one that 
takes a Context (and does not propose calls to startForResult) and one that 
takes an Activity (and allows calls to startForResult). I guess the second 
builder would be a subclass of the first.

I don't know if the "required" flag is so nice, because because can still start 
an activity the "standard" way, and there's no way to enforce that you did 
right at compile time. I guess we could also add a runtime throwing of 
exception when an activity is started and the required extra is not set. But 
maybe that's too much.

Anyway, it we go with the "required" param, I would rather add it to the first 
method call ("intent()") then the last.

Once again, very good idea!

Original comment by py.ricau on 30 Dec 2011 at 9:34

GoogleCodeExporter commented 8 years ago

Original comment by py.ricau on 30 Dec 2011 at 9:36

GoogleCodeExporter commented 8 years ago
I like the "intent" method name, it seems obvious afterwards, thanks.

It's indeed misleading to have a "required" extra which could not be set 
through the AndroidManifest.xml or through a user-defined intent. But I really 
think it could be useful for activities that are meant to be used with extras.

Rather than throw an exception, we could make the activity properly stop and 
result something, or maybe call a user-defined method, annotated with 
@MissingExtra. It could also be another annotation like 
@Required(errorCode=RESULT_CODE_IF_ABSENT).

Checking the extras is something the user *will* do, so I think it's worth 
digging.

But we actually don't have to do this at first glance, it should be another 
issue.

Original comment by jzap...@excilys.com on 30 Dec 2011 at 9:59

GoogleCodeExporter commented 8 years ago
Ok, I totally agree.

We'll leave the "required" part separated and implement it afterwards.

Original comment by py.ricau on 30 Dec 2011 at 10:16

GoogleCodeExporter commented 8 years ago
I gave it a few more thoughts. I think I'll drop the "startActivityForResult" 
part.

If we don't, it means we need two builder classes : one that takes a context 
and allows calls to start(), and one that takes an activity and allows calls to 
start() and startForResult().

Then, to avoid code duplication, you would want to define methods such as 
"flags" and "myDateExtra" on the standard builder class, and have another one 
that extends this class and adds startForResult() method.

In fact, the only proper and clean way to do that in Java is using Self Bounded 
Generics. Basically, we would have an "IntentBuilder<T extends IntentBuilder>" 
and a "ResultIntentBuilder extends IntentBuilder<ResultIntentBuilder>".

This would then allow to have methods in IntentBuilder defined the following 
way :

@IgnoreWarnings("unchecked_cast")
public T flags(int flags) {
  intent.setFlags(flags);
  return (T) this;
}

If we don't do this, we'll have to duplicate the IntentBuilder code.

That's a lot of work, furthermore I think most users don't get Self Bounded 
Generics, and might be afraid of it. Of course, it would make us look clever, 
but that's not the aim of AndroidAnnotations :-) .

I'm starting to think that maybe we should not care about 
startActivityForResult. We could still use the builder to build the intent.

IE :

startActivityForResult(MyActivity_.intent(this).flags(FLAG_ACTIVITY_CLEAR_TOP).m
yExtra("hello").get(), requestCode);

We could still do the standard one :

MyActivity_.intent(this).flags(FLAG_ACTIVITY_CLEAR_TOP).myExtra("hello").start()

Original comment by py.ricau on 2 Jan 2012 at 11:06

GoogleCodeExporter commented 8 years ago
Regarding Extras, we might very well be in trouble :-) .

I realized that with the new @Enhanced annotation, you can put @Extra on any 
bean. It will automatically extract the extra from the activity given to build 
the bean, if it's an activity :

   private void init_() {
        if (context_ instanceof Activity) {
            Activity activity = ((Activity) context_);
            Bundle extras_ = activity.getIntent().getExtras();
            if (extras_!= null) {
                if (extras_.containsKey("Test")) {
                    try {
                        testExtra = cast_(extras_.get("Test"));
                    } catch (ClassCastException e) {
                        Log.e("EnhancedClass_", "Could not cast extra to expected type, the field is left to its default value", e);
                    }
                }
            }

Should we restrain @Extra usage to activities ?

Original comment by py.ricau on 2 Jan 2012 at 11:12

GoogleCodeExporter commented 8 years ago
The final answer is no, we won't restrain @Extra usage. However, only extras on 
activities will lead to new intent builder methods.

Original comment by py.ricau on 2 Jan 2012 at 4:44

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 94284fb9bf1a.

Original comment by py.ricau on 2 Jan 2012 at 5:20