History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: RIFE-284
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Geert Bevin
Reporter: Steven Grimm
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
RIFE

@Exit annotation should be valid on fields

Created: 28/Jun/06 06:09 PM   Updated: 17/Feb/07 03:18 PM
Component/s: engine
Affects Version/s: 1.5
Fix Version/s: 1.5

Original Estimate: Unknown Remaining Estimate: Unknown Time Spent: Unknown
File Attachments: 1. Java Source File Annotations2ElementInfo.java (19 kb)
2. Java Source File ExitField.java (0.8 kb)
3. Java Source File FlowlinkField.java (2 kb)
4. Text File patch.txt (9 kb)
5. Java Source File Simple.java (4 kb)
6. Java Source File TestAnnotations2ElementInfo.java (9 kb)


Scale: Bite-sized


 Description  « Hide
I use string constants to reduce repetition of my exit names. Right now I have this:

@Elem(
    exits = {
        @Exit(name=Foo.EXIT_VIEW_FOO),
        @Exit(name=Foo.EXIT_ADD_FOO)
    }
)
public class Foo extends Element {
    public static final String EXIT_VIEW_FOO = "viewFoo";
    public static final String EXIT_ADD_FOO = "addFoo";
    ...
        exit(EXIT_ADD_FOO);
    ...
}

It would be convenient to get rid of those class-level annotations, something like this:

@Elem()
public class Foo extends Element {
    @Exit
    public static final String EXIT_VIEW_FOO = "viewFoo";
    @Exit
    public static final String EXIT_ADD_FOO = "addFoo";
    ...
        exit(EXIT_ADD_FOO);
    ...
}

 All   Comments   Work Log   Change History      Sort Order:
Geert Bevin [29/Jun/06 12:44 AM]
I wonder if this should not also be implemented for @File, @FileRegexp, @InBean, @InCookie, @Input, @OutBean, @OutCookie, @Output, @Param, @ParamRegexp, @Submission and @SubmissionBean then. Basically all the annotations with a name attribute. Sadly, we should come up with new names for them though since annotations can't have conditional attributes. Any idea for the names?

Geert Bevin [30/Jun/06 09:52 PM]
Actually, this is not that difficult to implement yourself. Add the new annotation interfaces to src/framework/com/uwyn/rife/engine/annotations and copy what's in their existing counterparts, just leaving out the name. Then update src/framework/com/uwyn/rife/engine/Annotations2ElementInfo.java to support the new annotations, just mimic what's their for the existing counterparts again.
Finally, add the new annotations to some additional fields of src/unittests/com/uwyn/rife/engine/testelements/annotations/Simple.java and update the test class src/unittests/com/uwyn/rife/engine/TestAnnotations2ElementInfo.java

That should do it.

Geert Bevin [08/Jul/06 03:29 PM]
I'm planning on releasing 1.5 next week, but will not have the time to implement this feature. Are you planning on implementing it? Otherwise I'll reschedule it for 1.6.

Steven Grimm [08/Jul/06 09:28 PM]
New files ExitField and FlowlinkField, plus modified version of Annotations2ElementInfo to use them.

Steven Grimm [08/Jul/06 09:30 PM]
Updated annotation test case to test @ExitField annotation. It doesn't look like the annotation unit test tests any cross-element stuff so I didn't add @FlowlinkField.

Geert Bevin [08/Jul/06 10:05 PM]
Thanks a lot for this contribution!

Looks like you didn't use the subversion version of the sources though. The sources of 1.5M2 are already quite dated. Would it be possible to adapt this for the latest sources? Also. if you use subversion, you can send me a patch, that's much easier for me to work with.

Geert Bevin [08/Jul/06 10:23 PM]
It's not that much work for me to adapt it myself. So if you have no time to checkout the repository and such, don't worry, just tell me.
It's good though to get you to adopt the 'standard' contribution method.

Steven Grimm [09/Jul/06 10:29 AM]
I had been using the 1.5M2 source snapshot since it works as a source attachment for the 1.5M2 binary distribution. Now that I'm able to successfully build RIFE from source I will switch to the Subversion version. Patches forthcoming.

Geert Bevin [09/Jul/06 10:40 AM]
About the cross-element tests, they are actually scattered throughout the entire engine test suite. I'll add them for this after I received your patch.

Steven Grimm [09/Jul/06 11:00 AM]
Attached a patch vs. the current Subversion trunk.

Steven Grimm [09/Jul/06 07:50 PM]
A couple of further changes to consider; these are more "how should it look to the developer?" things.

The name of the @FlowlinkField annotation is not necessarily the best. I considered @FlowlinkExitNameField and @FlowlinkSourceField too -- they might make the intended target of the annotation clearer. I opted for the shorter name, but maybe that's the wrong call.

One other thing I considered was combining these into one annotation -- if you specify a destination, it's a flowlink (with an implicitly generated exit), otherwise it's just an exit. I decided not to do that because it didn't seem consistent with the other annotations, but on the other hand there's something to be said for reducing the number of annotations people have to memorize.

Geert Bevin [10/Jul/06 12:40 AM]
Hmm, maybe @FlowlinkSrcExitField is indeed better. You never know what other fields you'd want to combine flowlink annotations with. Since you essentially remove the srcExit annotation attribute, the name makes a lot of sense imho.

What do you think?

I don't think that combining into one annotation would be good since then you'd have to make the parsing very dynamic and forgiven, and you move more errors to runtime instead of compile time.

Steven Grimm [10/Jul/06 01:05 AM]
I'm not sure I like abbreviating "Source" to "Src" there (it's long enough already that 3 extra characters doesn't cost much), but other than that, I'm fine with it.

Geert Bevin [10/Jul/06 07:30 AM]
Well, it mimics the srcExit attribute of the Flowlink annotation (or flowlink tag)

Steven Grimm [10/Jul/06 07:50 PM]
Fair enough.

Geert Bevin [11/Jul/06 01:56 PM]
Today @FlowlinkSrcExitField does indeed seem weird, thinking of @FlowlinkExitField now ... what do you think?