Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor #11

Open
mateor opened this issue Apr 3, 2013 · 0 comments
Open

Refactor #11

mateor opened this issue Apr 3, 2013 · 0 comments

Comments

@mateor
Copy link
Member

mateor commented Apr 3, 2013

Having gone through this code pretty thoroughly, even just for fixing style stuff, it is pretty obvious that a large portion of it was created by writing a method and then cut and paste. We can probably cut this down significantly if we wanted to. It works, but I think we could cut it down a lot.

And I think that we should want to. Even though the classes I am specifically thinking about are brand new Privacy classes (and therefore easily inserted) ones, I think we should do our absolute best to keep the amount of code we use to a minimum.

All of this style stuff isn't just being done for nit-picking. The reason that PDroid2.0 only works for CyanogenMod and OpenPDroid works for everything is that I went through it and fixed the style problems that were breaking the patches. That's it.

Plus, I have a bunch of different methods under consideration for how to port the patching process to the devices themselves. Some of you know that I have already been successful in patching a device in the Android terminal. But most of the real revolutionary stuff will only work if we are very very cheap about our invasion policy.

So yeah, this is here to inspire me to refactor a bunch of these cut and paste methods.

Edit: For example, we even have the same comments repeated in three sections about the exact same loop, one that has obviously just been copy-pasted three or four times into the class. That means we have the exact same 8-10 lines of comments repeated four times. That is unnecessary in and of itself, but I think we can try to factor out the copy/paste mentality that got us into that mess. See Instrumentation.java.
This (x4):

if (!allowIntent) {
                    //  test if broadcasting works! 
                    //  SM: I don't know what 'test if broadcasting works' means.
                    //  Send the notification intent
                    final Context tmp = who;
                    //  SM: Why is all of this done? 
                    //  It seems like a weirdly unnecessary bit of code...
                    new Thread(new Runnable() {
                        public void run() {
                            // SM: Not clear why there is this delay
                            try {
                                Thread.sleep(1000); //wait 1 Second
                            } catch(Exception e) {
                                //nothing here
                            }  
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant