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

@trace

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

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

@trace

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.

@JakeWharton

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.

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);
    }
}

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

@DayS

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 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.

@yDelouis

@yDelouis The Strings class has indeed complex methods, for example deepArrayToString(). 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 call toString().
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 logging Entering ....

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

@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.

@yDelouis

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

okay.

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

@WonderCsabo

@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: 0

Please 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? :)

@yDelouis

Why not? @yDelouis ?

updated the PR.

Sure, why not.

This was implemented. :)

Navigation Menu

Repository navigation

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

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

DayS commented on Dec 6, 2013

athkalia commented on Dec 6, 2013

dodgex commented on Aug 21, 2014

dodgex commented on Aug 21, 2014

dodgex commented on Aug 21, 2014

WonderCsabo commented on Aug 22, 2014

yDelouis commented on Aug 22, 2014

WonderCsabo commented on Aug 22, 2014

yDelouis commented on Aug 22, 2014

yDelouis commented on Aug 22, 2014

WonderCsabo commented on Aug 22, 2014

dodgex commented on Aug 22, 2014

WonderCsabo commented on Aug 22, 2014

dodgex commented on Aug 22, 2014

1 remaining item

WonderCsabo commented on Aug 23, 2014

dodgex commented on Aug 23, 2014

dodgex commented on Aug 23, 2014

WonderCsabo commented on Aug 23, 2014

dodgex commented on Aug 23, 2014

WonderCsabo commented on Aug 23, 2014

dodgex commented on Aug 23, 2014

yDelouis commented on Aug 23, 2014

WonderCsabo commented on Sep 22, 2014

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions