asterisk-java-users

Re: [Asterisk-java-users] Re: Problem with ReplyBuilderImpl.java

[Steve D. <Ste...@Su...>]

> What do you exactly need the raw reply for?

You answered it at the end of the next paragraph.

> When looking at the fastagi-test script supplied with asterisk the
> checkresult subroutine checks the status code and the result.
> All parts of the reply are available in AGIReply, so these two checks
> could be implemented using getStatusCode() == SC_SUCCESS and checking
> for getResult() to be a String representation of an Integer.
> The raw reply string is only printed in case of a failure.

Yep, and that is functionality not easily obtained.  Note there are 
other issues
that make a "port" difficult.  One is throwing an exception if status 
520 (or was it 510)
is returned.  Another, but only because it is time consuming, is 
listing the request
environment key/values.  Remember, I was doing this to help me learn
Asterisk-Java.  It pointed up some issues.  I'm not sure if they are 
important or not.

> If it is a real requirement to have the raw reply available I can 
> easily
> add it

I can't say it's a "real" requirement, more like a "nice to have".

>  as you suggested but as the purpose of asterisk-java is to hide
> the raw protocol from the Java developer I am not sure it really makes
> much sense.
> What do you think?

I see the value in that.  But I also think it's valuable to have access 
to the underlying
raw data in case someone needs it in an unanticipated way.

If I had my way, and I guess I could, I'd get rid of ReplyBuilder 
altogether.  I'd hand
the list of lines to AGIReply in it's constructor and let them stay 
cached in AGIReply,
giving access to (at least the first) line outside of the object.  The 
current getters would
become convenience methods that perform lazy evaluations if they are 
called.  Of
course the results are cached, not computed everytime.  No need for 
setters.  Frankly
I'm not sure why you chose to build the reply outside of the reply 
object.

One could probably get rid of RequestBuilder in the same way.

Perhaps I should put my thoughts into code and forward it to the 
mailing list for
consideration/discussion.

Thanks for a nice framework!

Steve

Re: [Asterisk-java-users] Re: Problem with ReplyBuilderImpl.java

[Stefan R. <sr...@re...>]

> > The raw reply string is only printed in case of a failure.
> Yep, and that is functionality not easily obtained.  Note there are=20
> other issues
> that make a "port" difficult.

Now I understand your issues. Porting existing AGI scripts from say Perl
to Java was not a scenario is looked at when designing Asterisk-java.

> One is throwing an exception if status 520 (or was it 510) is returned.
The exceptions for 510 and 520 were introduced to free the developer
from having to check the status code after every command sent to
Asterisk.

> Another, but only because it is time consuming, is listing the request
> environment key/values.=20
You are right access to the environment variables via a Map might indeed
be a nice convenience functionality especially in the development phase.

> If I had my way, and I guess I could, I'd get rid of ReplyBuilder=20
> altogether.  I'd hand
> the list of lines to AGIReply in it's constructor and let them stay=20
> cached in AGIReply,
> giving access to (at least the first) line outside of the object.  The=20
> current getters would
> become convenience methods that perform lazy evaluations if they are=20
> called.  Of
> course the results are cached, not computed everytime.  No need for=20
> setters.  Frankly
> I'm not sure why you chose to build the reply outside of the reply=20
> object.
> One could probably get rid of RequestBuilder in the same way.

The builder approach results from the implementation of the Manager API.
Building a ManagerEvent involves choosing the right event class and
filling the attributes via reflection. Therefore i choose to implement
the objects passed to Asterisk as dump value objects with no logic.
The FastAGI implementation does follow this approach for the AGIRequest
and AGIReply but not for the AGICommand. So it's not really consistent.

Your proposed approach would remove these two builders, remove the -
possible confusing - setters and allow easy access to the raw data.
It would also improve performance (though that might not be much of an
issue compared to the network communication involved).

Right now I don't see any drawbacks.

> Perhaps I should put my thoughts into code and forward it to the=20
> mailing list for consideration/discussion.

If you want to submit a patch I am happy to drop the builders for
FastAGI and follow your proposal.

=3DStefan