8000 fix receiver registered at onAttach by dodgex · Pull Request #1286 · androidannotations/androidannotations · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

fix receiver registered at onAttach #1286

Merged
WonderCsabo merged 2 commits into from
Mar 22, 2015
Merged

fix receiver registered at onAttach #1286

WonderCsabo merged 2 commits into from
Mar 22, 2015

Conversation

@dodgex
Copy link
Member
@dodgex dodgex commented Dec 23, 2014

see #1132

@dodgex
Copy link
Member Author
dodgex commented Dec 23, 2014

only caveat on this implementation is that the IntentFilter fields will get a own copy if they have the same registerAt value. but i think that should not be a big problem.

@dodgex
Copy link
Member Author
dodgex commented Dec 23, 2014

what i have thought about is adding some kind of class extending IntentFilter that has an constructor with two arrays for actions and dataSchemes that we don't have to use those fields.

eg.

private final IntentFilter intentFilter2_ = new IntentFilter();



intentFilter2_.addAction("org.androidannotations.ACTION_1");
getActivity().registerReceiver(onActionOnAttachReceiver_, intentFilter2_);

could become

getActivity().registerReceiver(onActionOnAttachReceiver_, new ExtendedIntentFilter(new String[] {"org.androidannotations.ACTION_1"}));

// or like this, but that would make the generation code more complex.
getActivity().registerReceiver(onActionOnAttachReceiver_, new ExtendedIntentFilter("org.androidannotations.ACTION_1"));

@yDelouis @WonderCsabo thoughts?

@WonderCsabo
Copy link
Member
WonderCsabo commented Dec 23, 2014

only caveat on this implementation is that the IntentFilter fields will get a own copy if they have the same registerAt value.

Can you add an example?

@dodgex
Copy link
Member Author
dodgex commented Dec 24, 2014

first let me start with a small correction. they get an own IntentFilter if they don't have the same registerAt value (but actions and schemes are the same).

the sample:
https://github.com/excilys/androidannotations/blob/develop/AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/receiver/FragmentWithReceiver.java

before this change there was only one IntentFilter field as all Receiver had the same actions and dataSchemes (empty)

private final IntentFilter intentFilter1_ = new IntentFilter();

but after this change, also the registerAt is taken care of and that results in 4 such fields in this sample. this could more or less easily be changed to only 2 fields by modifying the hashCode and equals of the IntentFilterData class that was introduced with the dataSchemes update. but for now i decided to keep these function with a default/non specialized behaviour.

@dodgex
Copy link
Member Author
dodgex commented Dec 24, 2014

with the suggested helper class this issue could go away and we wont have any IntentFilter fields at all.

@WonderCsabo
Copy link
Member
WonderCsabo commented Dec 24, 2014

I am not sure we should add "new" API classes for this. We should keep the generated code understandable, e.g. using only plain Android code in it.

@WonderCsabo
Copy link
Member
WonderCsabo commented Dec 27, 2014

Please rebase this.

@dodgex
Copy link
Member Author
dodgex commented Dec 28, 2014

rebased.

WonderCsabo
WonderCsabo reviewed Jan 8, 2015
View reviewed changes
...droidannotations/src/main/java/org/androidannotations/holder/ReceiverRegistrationHolder.java Outdated
Copy link
Member
@WonderCsabo WonderCsabo Jan 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using polymorphism and method overrides is a better design. Maybe you could create this method in EComponentHolder which just returns init body, and override it in EFragmentHolder to return the needed block?

Copy link
Member
@WonderCsabo WonderCsabo Jan 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dodgex did you miss this?

Copy link
Member Author
@dodgex dodgex Jan 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. sorry. i'll update the PR

@WonderCsabo
Copy link
Member
WonderCsabo commented Jan 8, 2015

Can you add a test?

@dodgex
Copy link
Member Author
dodgex commented Jan 8, 2015

yeah.

WonderCsabo
WonderCsabo reviewed Jan 8, 2015
View reviewed changes
...-test-1-5/src/test/java/org/androidannotations/test15/receiver/FragmentWithReceiverTest.java Outdated
Copy link
Member
@WonderCsabo WonderCsabo Jan 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the Test suffix from the method names.

Copy link
Member Author
@dodgex dodgex Jan 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops. done

WonderCsabo added a commit that referenced this pull request Mar 22, 2015
@WonderCsabo
Merge pull request #1286 from dodgex/1132_fix_receiver_registered_at_…
5e72dfe 
…onAttach

fix receiver registered at onAttach
@WonderCsabo WonderCsabo merged commit 5e72dfe into androidannotations:develop Mar 22, 2015
@WonderCsabo WonderCsabo added this to the 3.3 milestone Mar 22, 2015
@WonderCsabo
Copy link
Member
WonderCsabo commented Mar 22, 2015

FInally merged, thanks!

@dodgex dodgex deleted the 1132_fix_receiver_registered_at_onAttach branch March 25, 2015 08:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Enhancement

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

3 participants

@dodgex @WonderCsabo @yDelouis
0