Change the @Trace annotations to report on parameter values and return values

[athkalia]

Hello,
as mentioned here
https://groups.google.com/forum/#!topic/androidannotations/2BSDWe7r6OA
it would be quite useful for the @trace annotation to create a log statement with all the values of the parameters passed in the method if they are primitives, or the .toString() representations if they are objects. I guess that the same thing could happen with the return value of a method.

Best Regards,
Sakis

[DayS]

I agree. But what should happen if parameters are objects with big toString value ? Should we truncate the value ?

However, I'm planning this for 3.1

[athkalia]

Well if it’s big it’s usually cause it’s needed, so i’d say just leave it as a whole. You could also add a parameter to choose whether we want to log parameters or not. Also, I wonder if there is a way to see if the toString is actually implemented before using it, so that we don’t get the random Object@1943012390 values outputed.

Cheers,
Sakis

From: Damien
Sent: Friday, December 06, 2013 9:19 AM
To: excilys/androidannotations
Cc: athkalia
Subject: Re: [androidannotations] Change the @trace annotations to report on parameter values and return values (#797)

I agree. But what should happen if parameters are objects with big toString value ? Should we truncate the value ?

However, I'm planning this for 3.1

—
Reply to this email directly or view it on GitHub.

[dodgex]

I just started to work on this issue. Most of the code is done.

for the parameter values i would like to use the Strings class of JakeWharton/hugo.

a simple toString does not work for primitive types and for some unknown reason JVar.type().isPrimitive() returns false for primitive parameter types like int

as both projects are Licensed Apache 2.0 i think @JakeWharton would agree to use the class here. for sure i would add copyright headers mentioning him instead of eBusiness Information, Excilys Group like in the other AA files.

[dodgex]

when i'm allowed to use the Strings class, it would go to org.androidannotations.api.Strings as it is needed in runtime.

@EActivity
public class TestActivity extends Activity {
    @Trace
    protected void threeParams(int a, int b, String c) {
    }

    @Trace
    protected void noParams() {
    }
}

would produce (truncated)

import org.androidannotations.api.Strings;

@Override
public void noParams() {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        long start = System.currentTimeMillis();
        Log.i("TestActivity", ("Entering "+"[void noParams()]"));
        try {
            TestActivity_.super.noParams();
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [noParams()], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.noParams();
    }
}

@Override
public void threeParams(final int a, final int b, final String c) {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        long start = System.currentTimeMillis();
        Log.i("TestActivity", ("Entering "+ String.format("[void threeParams(a = %s, b = %s, c = %s)]", Strings.toString(a), Strings.toString(b), Strings.toString(c))));
        try {
            TestActivity_.super.threeParams(a, b, c);
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [threeParams(int,int,java.lang.String)], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.threeParams(a, b, c);
    }
}

[dodgex]

when i get an OK for using the Strings class and for the generated output, i'll create a PR for this changes.

[WonderCsabo]

Hmm, do we already have imported files with non-Excilys license headers? BTW, i really do not know the policy about this, hope @DayS can give us some advice here.

[yDelouis]

The license header will be overwritten by a mvn license:check. So, it's not so easy to include it.
What about generating the relevant part of Strings.toString() ?
I mean generating :

• '"' + str + '"', if the param str is a String
• Arrays.toString(intArray), if the param intArray is an array of int
• etc..

It would be more efficient and prevent us from managing two license.

[WonderCsabo]

@yDelouis The Strings class has indeed complex methods, for example deepArrayToString(). Is it a good idea to generate those?

[yDelouis]

I don't think we should handle complex cases like arrays of arrays.
It's not a common use case, won't be very readable in logs and will be slower at runtime.
I think we should handle primitive types, Strings, Arrays. For other parameters, we should call toString().
It will handle most of the cases and for others, the developer can still use a debugger.

[yDelouis]

A side note, long start = System.currentTimeMillis(); should be after logging Entering ....

[WonderCsabo]

I think you are right. For arrays, i guess you just want to call Arrays.toString, and do not care about nested arrays or such. If we do like you suggested, we do not have to include foreign code.

[dodgex]

@yDelouis @WonderCsabo ok, so we'll go with the simple way without the Strings class?

@yDelouis i haven't changed the calling order just added the paramerters. but i fix/can change it in my PR then.

[WonderCsabo]

I think yes, we should generate the calls like @yDelouis laid out in this comment.

[dodgex]

okay.

[dodgex]

damn :( i deleted my local+unpushed branch for this /cry

lets do it again :(

[WonderCsabo]

Do not panic. It is possible to still restore your branch locally. See here or here.

[dodgex]

@WonderCsabo you are my hero. <3

good that i was doing some other stuff instead of rebuilding the changes. :)

[dodgex]

@Trace public void helloWorld(){}
@Trace public void helloWorld(String test){}
@Trace public void helloWorld(int[] intArray){}
@Trace public void helloWorld(String test, int[] intArray){}
@Trace public void helloWorld(Intent intent){}

generates to:

@Override
public void helloWorld(final String test, final int[] intArray) {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        Log.i("TestActivity", ("Entering "+ String.format("[void helloWorld(test = %s, intArray = %s)]", test, Arrays.toString(intArray))));
        long start = System.currentTimeMillis();
        try {
            TestActivity_.super.helloWorld(test, intArray);
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [helloWorld(java.lang.String,int[])], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.helloWorld(test, intArray);
    }
}

@Override
public void helloWorld() {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        Log.i("TestActivity", ("Entering "+"[helloWorld()()]"));
        long start = System.currentTimeMillis();
        try {
            TestActivity_.super.helloWorld();
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [helloWorld()], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.helloWorld();
    }
}

@Override
public void helloWorld(final Intent intent) {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        Log.i("TestActivity", ("Entering "+ String.format("[void helloWorld(intent = %s)]", intent)));
        long start = System.currentTimeMillis();
        try {
            TestActivity_.super.helloWorld(intent);
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [helloWorld(android.content.Intent)], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.helloWorld(intent);
    }
}

@Override
public void helloWorld(final String test) {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        Log.i("TestActivity", ("Entering "+ String.format("[void helloWorld(test = %s)]", test)));
        long start = System.currentTimeMillis();
        try {
            TestActivity_.super.helloWorld(test);
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [helloWorld(java.lang.String)], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.helloWorld(test);
    }
}

@Override
public void helloWorld(final int[] intArray) {
    if (Log.isLoggable("TestActivity", Log.INFO)) {
        Log.i("TestActivity", ("Entering "+ String.format("[void helloWorld(intArray = %s)]", Arrays.toString(intArray))));
        long start = System.currentTimeMillis();
        try {
            TestActivity_.super.helloWorld(intArray);
        } finally {
            long duration = (System.currentTimeMillis()-start);
            Log.i("TestActivity", ("Exiting [helloWorld(int[])], duration in ms: "+ duration));
        }
    } else {
        TestActivity_.super.helloWorld(intArray);
    }
}

with this log output:

I/TestActivity﹕ Entering [void helloWorld()]
I/TestActivity﹕ Exiting [helloWorld()], duration in ms: 0
I/TestActivity﹕ Entering [void helloWorld(test = hello world)]
I/TestActivity﹕ Exiting [helloWorld(java.lang.String)], duration in ms: 41
I/TestActivity﹕ Entering [void helloWorld(intArray = [1, 2, 3])]
I/TestActivity﹕ Exiting [helloWorld(int[])], duration in ms: 0
I/TestActivity﹕ Entering [void helloWorld(test = hello world, intArray = [1, 2, 3])]
I/TestActivity﹕ Exiting [helloWorld(java.lang.String,int[])], duration in ms: 0
I/TestActivity﹕ Entering [void helloWorld(intent = Intent { act=THIS_IS_AN_ACTION dat=http://d.android.com })]
I/TestActivity﹕ Exiting [helloWorld(android.content.Intent)], duration in ms: 0

[WonderCsabo]

Please create a pull request, we discuss implementation there and will refactor if that is need.

[dodgex]

i just realized that this issue also requested to log the return value. should i implement that too? :)

[WonderCsabo]

Why not? @yDelouis ?

[dodgex]

updated the PR.

[yDelouis]

Sure, why not.

[WonderCsabo]

This was implemented. :)