These are chat archives for JnRouvignac/AutoRefactor

4th
Oct 2016
Luis Cruz
@luiscruz
Oct 04 2016 10:10
yes
my approach was creating a visitor to store the statement where the variable appears. Each time it appears this.lastCursorStatementis reassigned. In the end I expect that it stores the last use of the variable.
Unfortunately It takes a long time to run and then returns nothing
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 10:37
Hum
How about storing the last expression where it appears
Then only in the end look up the statement itself
Luis Cruz
@luiscruz
Oct 04 2016 10:40
sounds better, I’ll try it
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 10:41
Going up the tree all the time is costly
I expect
Luis Cruz
@luiscruz
Oct 04 2016 10:53
In this code sample
    public int ok(SQLiteDatabase db, long route_id, String table, String whereClause, String id) {
        int total_deletions = 0;
        Cursor cursor = db.query("TABLE_TRIPS",
                new String[]{
                        "KEY_TRIP_ID"},
                "ROUTE_ID" + "=?",
                new String[]{Long.toString(route_id)},
                null, null, null);
        while (cursor.moveToNext()) {
            total_deletions += db.delete(table, whereClause + "=?",
                    new String[]{Long.toString(cursor.getLong(0))});
        }
        return total_deletions;
    }
It is returning the node in the db.query() I would like to get the node in the while
(this is a test case I took from Android lint project; although it has a control flow statement I will assume it is an ordinay block)
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 10:57
Ok
What are you doing now?
Luis Cruz
@luiscruz
Oct 04 2016 10:59
    public class LastCursorChecker extends ASTVisitor {
        public SimpleName lastCursorUse;
        private SimpleName cursorSimpleName;

        public LastCursorChecker(SimpleName cursorSimpleName) {
            super();
            this.lastCursorUse = null;
            this.cursorSimpleName = cursorSimpleName;
        }

        @Override
        public boolean visit(SimpleName node) {
            if(ASTHelper.isSameLocalVariable(node, cursorSimpleName)){                
                this.lastCursorUse = node;
            }
            return VISIT_SUBTREE;
        }
    }
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 11:01
Is it not getting db.delete()?
Luis Cruz
@luiscruz
Oct 04 2016 11:03
nop
I did this ugly hack to debug
Block block = (Block) ASTNodes.getParent(cursorAssignmentExpressionStatement, ASTNode.BLOCK);
LastCursorChecker lastCursorChecker = new LastCursorChecker(cursorSimpleName);
block.accept(lastCursorChecker);
if(block != null){
    throw new IllegalArgumentException(""+lastCursorChecker.lastCursorUse.getParent()); 
}
and I get this:
java.lang.IllegalArgumentException: cursor=db.query("TABLE_TRIPS",new String[]{"KEY_TRIP_ID"},"ROUTE_ID=?",new String[]{Long.toString(route_id)},null,null,null)
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 11:08
Do you know you can debug from within eclipse ?
No printing necessary
It is explained in the various wiki pages
Luis Cruz
@luiscruz
Oct 04 2016 11:09
alright, I’ll have to check it :sweat_smile:
I’ve used the traditional way of creating breakpoints with the GUI and it didn’t work, so I figure out this ugly printing :sweat_smile:
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 11:10
"Run as junit plugin" or something like that
Make your life easy by sorting this out :)
Why does it not work then ?
Luis Cruz
@luiscruz
Oct 04 2016 11:12
debug mode set up ;)
Luis Cruz
@luiscruz
Oct 04 2016 11:18
I think I’ve found the issue...
Block block = (Block) ASTNodes.getParent(cursorAssignmentExpressionStatement, ASTNode.BLOCK);
I have to take a closer look to this
I really I have to go lunch now. I’ll get back to this in one hour
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 11:27
No worries
You should put a conditional breakpoint on the visit() method
Where node.getIdentifier().equals("delete")
Luis Cruz
@luiscruz
Oct 04 2016 15:03
done
:relieved:
now I should go into details: if the method returns a Cursor I should not add a close() call
Also I should check this only before the variable with the cursor is reassigned
The last one is tricky :worried:
Luis Cruz
@luiscruz
Oct 04 2016 15:17
anyway, so far all the lint use cases for this smell, are being tested with the current version of the code :)
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 15:59
Assignment is easy
Aliasing is not :)
Well done!
In two weeks time I think it will be good to try to merge some of these rules
(When I am done with work in the house)
Because I suspect we will rewrite some code :)
Make sure you assign copyright as appropriate
And that running mvn clean install builds successfully
Luis Cruz
@luiscruz
Oct 04 2016 16:07
Ok! For sure rewriting will be necessary 😅
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 16:11
I am very assertive when it comes to how code is written
Hence why I started this project :Présentation
The following step for you will be to run the rules on real code...
... and fix bugs
Luis Cruz
@luiscruz
Oct 04 2016 16:12
That's cool :) I'll have an intensive overview of the best coding practices
Alright
Jean-Noël Rouvignac
@JnRouvignac
Oct 04 2016 16:20
The best dunno, mine certainly :D