[sip-comm-dev] Documentation for FileAccessService


#1

Hello everybody,

The developer documentation for the FileAccessService
is out. You can check it here:
http://www.sip-communicator.org/index.php/Documentation/FileAccessService

As always, any comments are highly appreciated!

Regards,
Alex

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Hi Alex,

Alexander Pelov wrote:

Hello everybody,

The developer documentation for the FileAccessService
is out. You can check it here:
http://www.sip-communicator.org/index.php/Documentation/FileAccessService

As always, any comments are highly appreciated!

I like the way you document your Services :slight_smile:

But still some comments:

At the end of your four words explanation, you give an example of a path: /home/username/.sipcommunicator/history/filename.txt. It's maybe just a typo, but wouldn't it be better to have the SC directory called sip-communicator in order to avoid misunderstanding?

When reading a service documentation, I may be tempted to have a look to the corresponding Javadoc file, and would like to have an easy access to the autogenerated documentation from the service webpage. What do you think?

For the code section, I know there are several syntax highliters for pmwiki. I'll try to find one that fits.

Martin

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Martin Andr� wrote:

For the code section, I know there are several syntax highliters for pmwiki. I'll try to find one that fits.

I integrated the sourceblock cookbook in pmwiki and played a bit with the css to give a look close to what Eclipse does in its default behaviour.

You can view the result at
http://www.sip-communicator.org/index.php/Documentation/HistoryService
and compare with
http://www.sip-communicator.org/index.php/Documentation/FileAccessService

Is it any better?

Martin

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hey Alex,

I like that service and I think I'll be using quite often. Good job! I do have a few things to bring up though ( but you weren't expecting anything else, were you :wink: ).

Martin Andr� wrote:

At the end of your four words explanation, you give an example of a path: /home/username/.sipcommunicator/history/filename.txt. It's maybe just a typo, but wouldn't it be better to have the SC directory called sip-communicator in order to avoid misunderstanding?

I backup that.

Here are a few more from me:

1) One more thing. As far as properties go, we've been trying to keep a a naming convention according to whitch property preffixes should match the package they concern. So for the file access service this would give you:

net.java.sip.communicator.user.home

net.java.sip.communicator.user.home.sipdir

This would give you I know it's not written anywhere so excuse me for blurting it out like this.

2) Concerning the "net.java.sip.communicator.user.home.sipdir"

property. There's something in the "sipdir" part that troubles me. It makes me wonder whether it concerns sip-communicator in general or only its sip modules. I'd suggest something like scdir, sip-communicator-dir, sc.home, or anything that makes it clear we are talking about a property that influences global sip-communicator behaviour.

3) The CreatingServices page in the Dev Doc section says:

     The entry point to the service should carry the same name as the
     package followed by the service suffix.

Is there any specific reason why the package is not called fileaccess or why the service is not named ResoucesService.

4) In the "four words" section you say:

     We took an implementation that addresses those issues

It might be a good idea to acknowledge and link the source that we took that implementation from.

Once again, I like the service and think it will prove to be very handy!

When reading a service documentation, I may be tempted to have a look to the corresponding Javadoc file, and would like to have an easy access to the autogenerated documentation from the service webpage. What do you think?

I completely agree that this would be a nice thing to have but I'm afraid it's not as simple as it may appear. We're currently not storing javadocs on CVS. Javadocs are regenareted every time you build and this means that cvs would always detect modifications there and make you commit them, which on its turn would be a disaster for commit mails. Providing a link to CVS sourcecode insteand might be a good compromise though. WDYT?

For the code section, I know there are several syntax highliters for pmwiki. I'll try to find one that fits.

That's a great tip, Martin! I'd be very happy with a feature like that. I'd be writing docs day and night if that was the case ;).

Cheers
Emil

···

Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

> When reading a service documentation, I may be tempted to have a look to
> the corresponding Javadoc file, and would like to have an easy access to
> the autogenerated documentation from the service webpage. What do you think?

I completely agree that this would be a nice thing to have but I'm
afraid it's not as simple as it may appear. We're currently not storing
javadocs on CVS. Javadocs are regenareted every time you build and this
means that cvs would always detect modifications there and make you
commit them, which on its turn would be a disaster for commit mails.
Providing a link to CVS sourcecode insteand might be a good compromise
though. WDYT?

Why don't you take up a model for code documentation similar to what we
are doing in drupal. Have a look at http://drupaldocs.org. Fair enough,
that is done from withing a drupal module, which does the work what
javadoc is doing, but it should be possible to regenerate the javadocs
on a nightly/weekly/per-request basis, not keeping the in cvs. So you
could have an online reference always ready to go, maybe with a small
update lag. In drupal, that proved to be a very handy tool, and it
helped a lot budding developers and people willing to cut their teeth.

Cheers,
Vlado

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

There's one thing about all the asserts you use that bothers me a bit. I can see the utility of the tool from the implementor's point of view of course.

The thing is that I don't see how it makes code more reliable since it only moves a failure a bit earlier than it was supposed to happen. Assertion failures in both java and your tool extend an Error. Users of the FileAccessService are unlikely to try and catch assertion errors since they don't even have a way of knowing that such errors are thrown (they are not in javadoc and the compiler would let them go without being declared in a throws clause).

Let's take the getPrivatePersistentFile for example. In that particular case I'd find a java.lang.IllegalArgumentException declared in a throws clause to be much more appropriate and efficient.

Question 2, I vaguely remember discussing this a while ago but it would be kind of you to refresh my memory. Why do we hava our own assertion facility as opposed to the one that comes with 1.4?

Cheers
Emil

Emil Ivov wrote:

···

Hey Alex,

I like that service and I think I'll be using quite often. Good job! I do have a few things to bring up though ( but you weren't expecting anything else, were you :wink: ).

Martin Andr� wrote:

At the end of your four words explanation, you give an example of a path: /home/username/.sipcommunicator/history/filename.txt. It's maybe just a typo, but wouldn't it be better to have the SC directory called sip-communicator in order to avoid misunderstanding?

I backup that.

Here are a few more from me:

1) One more thing. As far as properties go, we've been trying to keep a a naming convention according to whitch property preffixes should match the package they concern. So for the file access service this would give you:

net.java.sip.communicator.user.home

net.java.sip.communicator.user.home.sipdir

This would give you I know it's not written anywhere so excuse me for blurting it out like this.

2) Concerning the "net.java.sip.communicator.user.home.sipdir"

property. There's something in the "sipdir" part that troubles me. It makes me wonder whether it concerns sip-communicator in general or only its sip modules. I'd suggest something like scdir, sip-communicator-dir, sc.home, or anything that makes it clear we are talking about a property that influences global sip-communicator behaviour.

3) The CreatingServices page in the Dev Doc section says:

     The entry point to the service should carry the same name as the
     package followed by the service suffix.

Is there any specific reason why the package is not called fileaccess or why the service is not named ResoucesService.

4) In the "four words" section you say:

     We took an implementation that addresses those issues

It might be a good idea to acknowledge and link the source that we took that implementation from.

Once again, I like the service and think it will prove to be very handy!

When reading a service documentation, I may be tempted to have a look to the corresponding Javadoc file, and would like to have an easy access to the autogenerated documentation from the service webpage. What do you think?

I completely agree that this would be a nice thing to have but I'm afraid it's not as simple as it may appear. We're currently not storing javadocs on CVS. Javadocs are regenareted every time you build and this means that cvs would always detect modifications there and make you commit them, which on its turn would be a disaster for commit mails. Providing a link to CVS sourcecode insteand might be a good compromise though. WDYT?

For the code section, I know there are several syntax highliters for pmwiki. I'll try to find one that fits.

That's a great tip, Martin! I'd be very happy with a feature like that. I'd be writing docs day and night if that was the case ;).

Cheers
Emil

Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#7

Hello Emil,

I'm glad that you mentioned the assertions - this way
we can discuss if this library should stay in the
project.

In my opinnion it should, but first the major
questions that should be answered:

1) Why use asserts?
  Using assertions is the Java way to define
Preconditions/Postconditions. For me the usage of
assertions improves the quality of code and helps
finding and fixing some nasty bugs.

2) Why not use Java 1.4's keyword "assert"?
  As we are also aiming to port (some day) SIP
Communicator to PDAs we should also support Java 1.3.
When this day comes we'll have to go through the code
and replace all assert-s. One may say that this is not
a big problem, because a simple "replace" would do the
work, BUT afterwards we'll get to the same point OR
we'll have to support two codebases..
  The biggest advantage (which is not implemented for
the moment) is the possibility to achieve a
Windows-like assertions. That is - whenever an
assertion fails, a window opens with the description,
the line where the error occured,.. well all
information we can. This window could ask the user
(developer) if she would like to continue the exection
of the program OR abort the execution of the program.
If there is no GUI, it may just abort the program thus
preserving the advantages of pure Java "assert".
  Also, the current implementation of
net.java.sip.communicator.util.Assert imposes a
human-readable explanation, in case of failure. A
simple assert(i%2 == 0); does only half of the job. An
Assert.assertTrue(i%2 == 0, "The number of senders
must be equal to the number of receivers") on the
other hand makes it pretty much obvious what is the
nature of the error.
  Why throw AssertError? To mimic the behaviour of
java "assert". Asserts are not supposed to be caught.
Normally they should never occur. A failed assertion
marks an error in the logic of the program. I would
definetly want to know if a sorting algorithm I used
does not work as advertised.
  Note: Instead of throw new AssertError(); one may
simply call assert(false);, which would have to be
replaced when going to Java 1.3.

Of course there may be some performance issues, but if
Assert-s are disabled the penalty is a function
execution + a single if statement.

As for some of the Asserts in the code - you are
absolutely right - asserts should not be used to
validate user input on public methods.. So I accept
the critics and I'll fix in a timely manner from
Asserts to "throw IllegalArgumentException".

Regards,
Alex

There's one thing about all the asserts you use that
bothers me a bit. I
can see the utility of the tool from the
implementor's point of view of
course.

The thing is that I don't see how it makes code more
reliable since it
only moves a failure a bit earlier than it was
supposed to happen.
Assertion failures in both java and your tool extend
an Error. Users of
the FileAccessService are unlikely to try and catch
assertion errors
since they don't even have a way of knowing that
such errors are thrown
(they are not in javadoc and the compiler would let
them go without
being declared in a throws clause).

Let's take the getPrivatePersistentFile for example.
In that particular
case I'd find a java.lang.IllegalArgumentException
declared in a throws
clause to be much more appropriate and efficient.

Question 2, I vaguely remember discussing this a
while ago but it would
be kind of you to refresh my memory. Why do we hava
our own assertion
facility as opposed to the one that comes with 1.4?

Cheers
Emil

Emil Ivov wrote:
> Hey Alex,
>
> I like that service and I think I'll be using
quite often. Good job! I
> do have a few things to bring up though ( but you
weren't expecting
> anything else, were you :wink: ).
>
> Martin Andr� wrote:
>
>>At the end of your four words explanation, you
give an example of a
>>path:

/home/username/.sipcommunicator/history/filename.txt.

···

--- Emil Ivov <emil_ivov@yahoo.com> wrote:

It's maybe
>>just a typo, but wouldn't it be better to have the
SC directory called
>>sip-communicator in order to avoid
misunderstanding?
>
>
> I backup that.
>
> Here are a few more from me:
>
> 1) One more thing. As far as properties go, we've
been trying to keep a
> a naming convention according to whitch property
preffixes should match
> the package they concern. So for the file access
service this would give
> you:
>
> net.java.sip.communicator.user.home
>
> net.java.sip.communicator.user.home.sipdir
>
> This would give you I know it's not written
anywhere so excuse me for
> blurting it out like this.
>
> 2) Concerning the
"net.java.sip.communicator.user.home.sipdir"
>
> property. There's something in the "sipdir" part
that troubles me. It
> makes me wonder whether it concerns
sip-communicator in general or only
> its sip modules. I'd suggest something like scdir,
sip-communicator-dir,
> sc.home, or anything that makes it clear we are
talking about a property
> that influences global sip-communicator behaviour.
>
> 3) The CreatingServices page in the Dev Doc
section says:
>
> The entry point to the service should carry
the same name as the
> package followed by the service suffix.
>
> Is there any specific reason why the package is
not called fileaccess or
> why the service is not named ResoucesService.
>
> 4) In the "four words" section you say:
>
> We took an implementation that addresses
those issues
>
> It might be a good idea to acknowledge and link
the source that we took
> that implementation from.
>
> Once again, I like the service and think it will
prove to be very handy!
>
>
>>When reading a service documentation, I may be
tempted to have a look to
>>the corresponding Javadoc file, and would like to
have an easy access to
>>the autogenerated documentation from the service
webpage. What do you think?
>
>
> I completely agree that this would be a nice thing
to have but I'm
> afraid it's not as simple as it may appear. We're
currently not storing
> javadocs on CVS. Javadocs are regenareted every
time you build and this
> means that cvs would always detect modifications
there and make you
> commit them, which on its turn would be a disaster
for commit mails.
> Providing a link to CVS sourcecode insteand might
be a good compromise
> though. WDYT?
>
>
>>For the code section, I know there are several
syntax highliters for
>>pmwiki. I'll try to find one that fits.
>
>
> That's a great tip, Martin! I'd be very happy with
a feature like that.
> I'd be writing docs day and night if that was the
case ;).
>
> Cheers
> Emil
>
>
>>Martin
>>

---------------------------------------------------------------------

>>To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
>>For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
>>
>>
>
>
>

---------------------------------------------------------------------

> To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
> For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net
>
>

---------------------------------------------------------------------

To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net


#8

vlado wrote:

Why don't you take up a model for code documentation similar to what we are doing in drupal. Have a look at http://drupaldocs.org. Fair enough,
that is done from withing a drupal module, which does the work what
javadoc is doing, but it should be possible to regenerate the javadocs
on a nightly/weekly/per-request basis, not keeping the in cvs. So you
could have an online reference always ready to go, maybe with a small
update lag. In drupal, that proved to be a very handy tool, and it
helped a lot budding developers and people willing to cut their teeth.

Great tip! I'll make the nightly builds build javadocs too and upload them somewhere.

Thanks Vlado

Emil

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#9

Alexander Pelov wrote:

1) Why use asserts? Using assertions is the Java way to define Preconditions/Postconditions. For me the usage of assertions improves
the quality of code and helps finding and fixing some nasty bugs.

Yes and yet one should be very careful not to use asserts for error handling and only keep it for cases that represent "undefined program state".

2) Why not use Java 1.4's keyword "assert"? As we are also aiming to
port (some day) SIP Communicator to PDAs we should also support Java
1.3.

Oh yes. I remember that one. Well this is not the case any more since
CDC 1.1 and the new PersonalProfile are based on 1.4.

The biggest advantage (which is not implemented for the moment) is
the possibility to achieve a Windows-like assertions. That is -
whenever an assertion fails, a window opens with the description, the
line where the error occured,.. well all information we can. This
window could ask the user (developer) if she would like to continue
the exection of the program OR abort the execution of the program.

That's a point. Yet, are we about to implement that? Does that justify
all the disadvantages of implementing our own assertion utility?

net.java.sip.communicator.util.Assert imposes a human-readable
explanation, in case of failure.

Yes I noticed that. It seems like a good idea. I am afraid though that
this may tempt developers to use the lib for purposes that is not meant
for such as the examples I gave for the FileAccessServiceImpl.

Of course there may be some performance issues, but if Assert-s are
disabled the penalty is a function execution + a single if statement.

Well it appears that performance was one of the reasons that the assert keyword was added to java.

See "Why does this facility justify a language change, as opposed to a
library solution?" here:

http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html

Of course we don't care that (a < b) in

To summarize, I believe that we don't really need the Assertion utility.
What bothers me most is that many would start using it for error
handling which is inherently wrong. Right now for example a search
indicates 24 usage occurrences of Assert.assert() in sip-communicator.
Out of these 24 I saw only one that really corresponds to an assert
statement:

FileAccessService.getFullPath(String):
...
     Assert.assertNonNull(
         this.configurationService,
         "The configurationService should be non-null.");
...
and the java assert key word would do just fine here.

All the rest are used for error handling.

Cheers
Emil

···

As for some of the Asserts in the code - you are absolutely right -
asserts should not be used to validate user input on public methods..
So I accept the critics and I'll fix in a timely manner from Asserts
to "throw IllegalArgumentException".

Regards, Alex

--- Emil Ivov <emil_ivov@yahoo.com> wrote:

There's one thing about all the asserts you use that bothers me a
bit. I can see the utility of the tool from the implementor's point
of view of course.

The thing is that I don't see how it makes code more reliable since
it only moves a failure a bit earlier than it was supposed to
happen. Assertion failures in both java and your tool extend an
Error. Users of the FileAccessService are unlikely to try and catch
assertion errors since they don't even have a way of knowing that such errors are thrown (they are not in javadoc and the compiler
would let them go without being declared in a throws clause).

Let's take the getPrivatePersistentFile for example. In that
particular case I'd find a java.lang.IllegalArgumentException declared in a throws clause to be much more appropriate and
efficient.

Question 2, I vaguely remember discussing this a while ago but it
would be kind of you to refresh my memory. Why do we hava our own
assertion facility as opposed to the one that comes with 1.4?

Cheers Emil

Emil Ivov wrote:

Hey Alex,

I like that service and I think I'll be using

quite often. Good job! I

do have a few things to bring up though ( but you

weren't expecting

anything else, were you :wink: ).

Martin Andr� wrote:

At the end of your four words explanation, you

give an example of a

path:

/home/username/.sipcommunicator/history/filename.txt.

It's maybe

just a typo, but wouldn't it be better to have the

SC directory called

sip-communicator in order to avoid

misunderstanding?

I backup that.

Here are a few more from me:

1) One more thing. As far as properties go, we've

been trying to keep a

a naming convention according to whitch property

preffixes should match

the package they concern. So for the file access

service this would give

you:

net.java.sip.communicator.user.home

net.java.sip.communicator.user.home.sipdir

This would give you I know it's not written

anywhere so excuse me for

blurting it out like this.

2) Concerning the

"net.java.sip.communicator.user.home.sipdir"

property. There's something in the "sipdir" part

that troubles me. It

makes me wonder whether it concerns

sip-communicator in general or only

its sip modules. I'd suggest something like scdir,

sip-communicator-dir,

sc.home, or anything that makes it clear we are

talking about a property

that influences global sip-communicator behaviour.

3) The CreatingServices page in the Dev Doc

section says:

The entry point to the service should carry

the same name as the

package followed by the service suffix.

Is there any specific reason why the package is

not called fileaccess or

why the service is not named ResoucesService.

4) In the "four words" section you say:

We took an implementation that addresses

those issues

It might be a good idea to acknowledge and link

the source that we took

that implementation from.

Once again, I like the service and think it will

prove to be very handy!

When reading a service documentation, I may be

tempted to have a look to

the corresponding Javadoc file, and would like to

have an easy access to

the autogenerated documentation from the service

webpage. What do you think?

I completely agree that this would be a nice thing

to have but I'm

afraid it's not as simple as it may appear. We're

currently not storing

javadocs on CVS. Javadocs are regenareted every

time you build and this

means that cvs would always detect modifications

there and make you

commit them, which on its turn would be a disaster

for commit mails.

Providing a link to CVS sourcecode insteand might

be a good compromise

though. WDYT?

For the code section, I know there are several

syntax highliters for

pmwiki. I'll try to find one that fits.

That's a great tip, Martin! I'd be very happy with

a feature like that.

I'd be writing docs day and night if that was the

case ;).

Cheers Emil

Martin

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net For additional
commands, e-mail: dev-help@sip-communicator.dev.java.net


#10

Alexander Pelov wrote:

1) Why use asserts? Using assertions is the Java way to define Preconditions/Postconditions. For me the usage of assertions improves
the quality of code and helps finding and fixing some nasty bugs.

Yes and yet one should be very careful not to use asserts for error
handling and only keep it for cases that represent "undefined program
state".

2) Why not use Java 1.4's keyword "assert"? As we are also aiming to
port (some day) SIP Communicator to PDAs we should also support Java
1.3.

Oh yes. I remember that one. Well this is not the case any more since
CDC 1.1 and the new PersonalProfile are based on 1.4.

The biggest advantage (which is not implemented for the moment) is
the possibility to achieve a Windows-like assertions. That is -
whenever an assertion fails, a window opens with the description, the
line where the error occured,.. well all information we can. This
window could ask the user (developer) if she would like to continue
the exection of the program OR abort the execution of the program.

That's a point. Yet, are we about to implement that? Does that justify
all the disadvantages of implementing our own assertion utility?

net.java.sip.communicator.util.Assert imposes a human-readable
explanation, in case of failure.

Yes I noticed that. It seems like a good idea. I am afraid though that
this may tempt developers to use the lib for purposes that is not meant
for such as the examples I gave for the FileAccessServiceImpl.

Of course there may be some performance issues, but if Assert-s are
disabled the penalty is a function execution + a single if statement.

Well it appears that performance was one of the reasons that the assert
keyword was added to java.

See "Why does this facility justify a language change, as opposed to a
library solution?" here:

http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html

Of course we don't care that (a < b) in

To summarize, I believe that we don't really need the Assertion utility.
What bothers me most is that many would start using it for error
handling which is inherently wrong. Right now for example a search
indicates 24 usage occurrences of Assert.assert() in sip-communicator.
Out of these 24 I saw only one that really corresponds to an assert
statement:

FileAccessService.getFullPath(String):
...
      Assert.assertNonNull(
          this.configurationService,
          "The configurationService should be non-null.");
...
and the java assert key word would do just fine here.

All the rest are used for error handling.

Cheers
Emil

···

As for some of the Asserts in the code - you are absolutely right -
asserts should not be used to validate user input on public methods..
So I accept the critics and I'll fix in a timely manner from Asserts
to "throw IllegalArgumentException".

Regards, Alex

--- Emil Ivov <emil_ivov@yahoo.com> wrote:

There's one thing about all the asserts you use that bothers me a
bit. I can see the utility of the tool from the implementor's point
of view of course.

The thing is that I don't see how it makes code more reliable since
it only moves a failure a bit earlier than it was supposed to
happen. Assertion failures in both java and your tool extend an
Error. Users of the FileAccessService are unlikely to try and catch
assertion errors since they don't even have a way of knowing that such errors are thrown (they are not in javadoc and the compiler
would let them go without being declared in a throws clause).

Let's take the getPrivatePersistentFile for example. In that
particular case I'd find a java.lang.IllegalArgumentException declared in a throws clause to be much more appropriate and
efficient.

Question 2, I vaguely remember discussing this a while ago but it
would be kind of you to refresh my memory. Why do we hava our own
assertion facility as opposed to the one that comes with 1.4?

Cheers Emil

Emil Ivov wrote:

Hey Alex,

I like that service and I think I'll be using

quite often. Good job! I

do have a few things to bring up though ( but you

weren't expecting

anything else, were you :wink: ).

Martin Andr� wrote:

At the end of your four words explanation, you

give an example of a

path:

/home/username/.sipcommunicator/history/filename.txt.

It's maybe

just a typo, but wouldn't it be better to have the

SC directory called

sip-communicator in order to avoid

misunderstanding?

I backup that.

Here are a few more from me:

1) One more thing. As far as properties go, we've

been trying to keep a

a naming convention according to whitch property

preffixes should match

the package they concern. So for the file access

service this would give

you:

net.java.sip.communicator.user.home

net.java.sip.communicator.user.home.sipdir

This would give you I know it's not written

anywhere so excuse me for

blurting it out like this.

2) Concerning the

"net.java.sip.communicator.user.home.sipdir"

property. There's something in the "sipdir" part

that troubles me. It

makes me wonder whether it concerns

sip-communicator in general or only

its sip modules. I'd suggest something like scdir,

sip-communicator-dir,

sc.home, or anything that makes it clear we are

talking about a property

that influences global sip-communicator behaviour.

3) The CreatingServices page in the Dev Doc

section says:

The entry point to the service should carry

the same name as the

package followed by the service suffix.

Is there any specific reason why the package is

not called fileaccess or

why the service is not named ResoucesService.

4) In the "four words" section you say:

We took an implementation that addresses

those issues

It might be a good idea to acknowledge and link

the source that we took

that implementation from.

Once again, I like the service and think it will

prove to be very handy!

When reading a service documentation, I may be

tempted to have a look to

the corresponding Javadoc file, and would like to

have an easy access to

the autogenerated documentation from the service

webpage. What do you think?

I completely agree that this would be a nice thing

to have but I'm

afraid it's not as simple as it may appear. We're

currently not storing

javadocs on CVS. Javadocs are regenareted every

time you build and this

means that cvs would always detect modifications

there and make you

commit them, which on its turn would be a disaster

for commit mails.

Providing a link to CVS sourcecode insteand might

be a good compromise

though. WDYT?

For the code section, I know there are several

syntax highliters for

pmwiki. I'll try to find one that fits.

That's a great tip, Martin! I'd be very happy with

a feature like that.

I'd be writing docs day and night if that was the

case ;).

Cheers Emil

Martin

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail:

dev-unsubscribe@sip-communicator.dev.java.net

For additional commands, e-mail:

dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net For additional
commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#11

Hi Emil,

You're absolutely right that in the FileAccessService
I have missused the Assert-ions (as your search
shows). I didn't know that the new CDC will include
assertions, so this point is off. Given this I also
agree that although there are some advantages it would
be better to drop the Assert class.

As I analysed my serious mistakes with the usage of
the Assert class I came to realize that there actually
is a place for some common mechanism allowing to
validate user input. Something like ParamValidator,
which throws appropriate exceptions (like
InvalidArgumentException). This way the developer may
call ParamValidator.requreNonNull(param, "Param should
be non-null."); I think that such a convinience would
encourage the programmers to pay more attention to
function parameters. What do you think on this?

Regards,
Alex

Alexander Pelov wrote:
> 1) Why use asserts? Using assertions is the Java
way to define
> Preconditions/Postconditions. For me the usage of
assertions improves
> the quality of code and helps finding and fixing
some nasty bugs.

Yes and yet one should be very careful not to use
asserts for error
handling and only keep it for cases that represent
"undefined program
state".

>
> 2) Why not use Java 1.4's keyword "assert"? As we
are also aiming to
> port (some day) SIP Communicator to PDAs we should
also support Java
> 1.3.

Oh yes. I remember that one. Well this is not the
case any more since
CDC 1.1 and the new PersonalProfile are based on
1.4.

> The biggest advantage (which is not implemented
for the moment) is
> the possibility to achieve a Windows-like
assertions. That is -
> whenever an assertion fails, a window opens with
the description, the
> line where the error occured,.. well all
information we can. This
> window could ask the user (developer) if she would
like to continue
> the exection of the program OR abort the execution
of the program.

That's a point. Yet, are we about to implement that?
Does that justify
all the disadvantages of implementing our own
assertion utility?

> net.java.sip.communicator.util.Assert imposes a
human-readable
> explanation, in case of failure.

Yes I noticed that. It seems like a good idea. I am
afraid though that
this may tempt developers to use the lib for
purposes that is not meant
for such as the examples I gave for the
FileAccessServiceImpl.

> Of course there may be some performance issues,
but if Assert-s are
> disabled the penalty is a function execution + a
single if statement.
>
Well it appears that performance was one of the
reasons that the assert
keyword was added to java.

See "Why does this facility justify a language
change, as opposed to a
library solution?" here:

http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html

Of course we don't care that (a < b) in

To summarize, I believe that we don't really need
the Assertion utility.
What bothers me most is that many would start using
it for error
handling which is inherently wrong. Right now for
example a search
indicates 24 usage occurrences of Assert.assert() in
sip-communicator.
Out of these 24 I saw only one that really
corresponds to an assert
statement:

FileAccessService.getFullPath(String):
...
      Assert.assertNonNull(
          this.configurationService,
          "The configurationService should be
non-null.");
...
and the java assert key word would do just fine
here.

All the rest are used for error handling.

Cheers
Emil

> As for some of the Asserts in the code - you are
absolutely right -
> asserts should not be used to validate user input
on public methods..
> So I accept the critics and I'll fix in a timely
manner from Asserts
> to "throw IllegalArgumentException".
>
> Regards, Alex
>
>
>
>> There's one thing about all the asserts you use
that bothers me a
>> bit. I can see the utility of the tool from the
implementor's point
>> of view of course.
>>
>> The thing is that I don't see how it makes code
more reliable since
>> it only moves a failure a bit earlier than it was
supposed to
>> happen. Assertion failures in both java and your
tool extend an
>> Error. Users of the FileAccessService are
unlikely to try and catch
>> assertion errors since they don't even have a
way of knowing that
>> such errors are thrown (they are not in javadoc
and the compiler
>> would let them go without being declared in a
throws clause).
>>
>> Let's take the getPrivatePersistentFile for
example. In that
>> particular case I'd find a
java.lang.IllegalArgumentException
>> declared in a throws clause to be much more
appropriate and
>> efficient.
>>
>> Question 2, I vaguely remember discussing this a
while ago but it
>> would be kind of you to refresh my memory. Why do
we hava our own
>> assertion facility as opposed to the one that
comes with 1.4?
>>
>> Cheers Emil
>>
>> Emil Ivov wrote:
>>
>>> Hey Alex,
>>>
>>> I like that service and I think I'll be using
>>
>> quite often. Good job! I
>>
>>> do have a few things to bring up though ( but
you
>>
>> weren't expecting
>>
>>> anything else, were you :wink: ).
>>>
>>> Martin Andr� wrote:
>>>
>>>
>>>> At the end of your four words explanation, you
>>
>> give an example of a
>>
>>>> path:
>>
>

/home/username/.sipcommunicator/history/filename.txt.

···

--- Emil Ivov <emil_ivov@yahoo.com> wrote:

> --- Emil Ivov <emil_ivov@yahoo.com> wrote:
>
>> It's maybe
>>
>>>> just a typo, but wouldn't it be better to have
the
>>
>> SC directory called
>>
>>>> sip-communicator in order to avoid
>>
>> misunderstanding?
>>
>>>
>>> I backup that.
>>>
>>> Here are a few more from me:
>>>
>>> 1) One more thing. As far as properties go,
we've
>>
>> been trying to keep a
>>
>>> a naming convention according to whitch property
>>
>> preffixes should match
>>
>>> the package they concern. So for the file access
>>
>> service this would give
>>
>>> you:
>>>
>>> net.java.sip.communicator.user.home
>>>
>>> net.java.sip.communicator.user.home.sipdir
>>>
>>> This would give you I know it's not written
>>
>> anywhere so excuse me for
>>
>>> blurting it out like this.
>>>
>>> 2) Concerning the
>>
>> "net.java.sip.communicator.user.home.sipdir"
>>
>>> property. There's something in the "sipdir" part
>>
>> that troubles me. It
>>
>>> makes me wonder whether it concerns
>>
>> sip-communicator in general or only
>>
>>> its sip modules. I'd suggest something like
scdir,
>>
>> sip-communicator-dir,
>>
>>> sc.home, or anything that makes it clear we are
>>
>> talking about a property
>>
>>> that influences global sip-communicator
behaviour.
>>>
>>> 3) The CreatingServices page in the Dev Doc
>>
>> section says:
>>
>>> The entry point to the service should carry
>>
>> the same name as the
>>
>>> package followed by the service suffix.
>>>
>>> Is there any specific reason why the package is
>>
>> not called fileaccess or
>>
>>> why the service is not named ResoucesService.
>>>
>>> 4) In the "four words" section you say:
>>>
>>> We took an implementation that addresses
>>
>> those issues
>>
>>> It might be a good idea to acknowledge and link
>>
>> the source that we took
>>
>>> that implementation from.
>>>
>>> Once again, I like the service and think it will
>>
>> prove to be very handy!
>>
>>>
>>>> When reading a service documentation, I may be
>>
>> tempted to have a look to
>>
>>>> the corresponding Javadoc file, and would like
to
>>
>> have an easy access to
>>
>>>> the autogenerated documentation from the
service
>>
>> webpage. What do you think?
>>
>>>
>>> I completely agree that this would be a nice
thing
>>
>> to have but I'm
>>
>>> afraid it's not as simple as it may appear.
We're
>>
>> currently not storing
>>
>>> javadocs on CVS. Javadocs are regenareted every
>>
>> time you build and this
>>
>>> means that cvs would always detect modifications
>>
>> there and make you
>>
>>> commit them, which on its turn would be a
disaster
>>
>> for commit mails.
>>
>>> Providing a link to CVS sourcecode insteand
might
>>
>> be a good compromise
>>
>>> though. WDYT?
>>>
>>>
>>>
>>>> For the code section, I know there are several
>>
>> syntax highliters for
>>
>>>> pmwiki. I'll try to find one that fits.
>>>
>>>
>>> That's a great tip, Martin! I'd be very happy
with
>>
>> a feature like that.
>>
>>> I'd be writing docs day and night if that was
the
>>
>> case ;).
>>
>>> Cheers Emil
>>>
>>>
>>>
>>>> Martin
>>>>
>>
>>>

---------------------------------------------------------------------

>>>
>>>
>>>> To unsubscribe, e-mail:
>>
>> dev-unsubscribe@sip-communicator.dev.java.net
>>
>>>> For additional commands, e-mail:
>>
>> dev-help@sip-communicator.dev.java.net
>>
>>>>
>>>
>>>
>

---------------------------------------------------------------------

>
>
>>> To unsubscribe, e-mail:
>>
>> dev-unsubscribe@sip-communicator.dev.java.net
>>
>>> For additional commands, e-mail:
>>
>> dev-help@sip-communicator.dev.java.net
>>
>>>
>>
>

---------------------------------------------------------------------

>
>
>> To unsubscribe, e-mail:
>> dev-unsubscribe@sip-communicator.dev.java.net For
additional
>> commands, e-mail:
dev-help@sip-communicator.dev.java.net
>>
>>
>
>
>

---------------------------------------------------------------------

To unsubscribe, e-mail:
dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail:
dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net