Repository navigation
Change the @Trace annotations to report on parameter values and return values #797
Description
Activity
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
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,
SakisFrom: 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.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
toStringdoes not work for primitive types and for some unknown reasonJVar.type().isPrimitive()returns false for primitive parameter types likeintas 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 Grouplike in the other AA files.when i'm allowed to use the Strings class, it would go to
org.androidannotations.api.Stringsas 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); } }
when i get an OK for using the Strings class and for the generated output, i'll create a PR for this changes.
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.
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 ofStrings.toString()?
I mean generating :'"' + str + '"', if the paramstris aStringArrays.toString(intArray), if the paramintArrayis anarrayofint- etc..
It would be more efficient and prevent us from managing two license.
@yDelouis The
Stringsclass has indeed complex methods, for exampledeepArrayToString(). Is it a good idea to generate those?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 calltoString().
It will handle most of the cases and for others, the developer can still use a debugger.A side note,
long start = System.currentTimeMillis();should be after loggingEntering ....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.@yDelouis @WonderCsabo ok, so we'll go with the simple way without the
Stringsclass?@yDelouis i haven't changed the calling order just added the paramerters. but i fix/can change it in my PR then.
okay.
1 remaining item
@WonderCsabo you are my hero. <3
good that i was doing some other stuff instead of rebuilding the changes. :)
@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: 0Please create a pull request, we discuss implementation there and will refactor if that is need.
i just realized that this issue also requested to log the return value. should i implement that too? :)
Why not? @yDelouis ?
updated the PR.
Sure, why not.
This was implemented. :)
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