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

adding the ability to evaluate groovy expressions #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

orlandobcrra
Copy link
Contributor

No description provided.

@orlandobcrra
Copy link
Contributor Author

Hi @jakubnabrdalik,

What do you think about this one?

Example:

    //given
    SampleObject testObject = new SampleObject()
    def propertyExpressions = [
        '',
        'it.stringValue', 
        'it.booleanValue?" it is true ":"it is false "',
        'it.shortValue + it.integerValue',
        'Math.sqrt(it.doubleValue)'
    ]

    //when
    xlsxReporter.add(testObject, propertyExpressions, 0, true)

@jakubnabrdalik
Copy link
Collaborator

Oralndo, IMHO this is a wrong way to do it in groovy. This is programming in strings. No help from IDE, compiler or anything.

The Groovy-way would be to use closures (or functional interface and lambdas in Java8).

So I would suggest something like:

Map properties = [
   { it.toString() },
   { it.shortValue + it.integerValue },
   { Math.sqrt(it.doubleValue) }
]

This has the advantage of more IDE/compiler support.

To do that, we should introduce a new interface, because interface Getter is not a functional interface. If you feel like doing this, it would be great. Otherwise I can do it, but it's gonna take me some time (lots of work at the moment).

@orlandobcrra
Copy link
Contributor Author

Hi @jakubnabrdalik,

I removed the ugly string evaluation :) and pushed a commit to handle Closures, it was more easy, actually.
So, what do you think now?
https://github.com/TouK/excel-export/pull/21/files

@jakubnabrdalik
Copy link
Collaborator

Ok, it's gonna be a bit more complex.
You have extended PropertyGetter while you should just implement Getter, because you don't have a property in closure, you are working on the object directly. But that's not a big problem. The problem is that currently Getter has two methods, one returning property name, and the other formatting that property. That's bad for a closure, which doesn't have a property name (which is why you returned null). And you cannot return a null there, because it's a list that should not have nulls (it works by accident). The whole Getter has to be refactored, but the problem is, I'd have to break backward compatibility for this. Which, perhaps, is a way to go.
I have to think a bit more on that. I see how the plugin could be rewritten better, so may it's time for 1.0.

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

Successfully merging this pull request may close these issues.

2 participants