Re: FYI: Patch: java.net: socket stuff

[Dalibor Topic <robilad at kaffe dot org>]

Michael Koch wrote:
> Hi list,
>
>
> I commited the attached big patch to fix several issues in the socket stuff. 
> This mainly adds checks to many methods to see if the socket is closed 
> already.

I like the patch, though I'm not a gcj developer. I have one small nit 
to pick, though. Instead of copying

    if (isClosed())
      throw new SocketException("socket is closed");

so many times, you could have used a private method to do the checking 
and throwing.

	checkIfIsClosed();

private void checkIfIsClosed() throws SocketException {
   if (isClosed())
      throw new SocketException("socket is closed");
}

Code duplication is evil ;)

I've got another small wish: as you've been wrestling with the mauve 
tests for DatagramSocket, and managed to understand them, maybe could 
you clean them up, and separate them into one file per tested method/API.

That would make it much clearer what's being tested. I'll have my 
FieldPosition mauve test refactoring & rewriting done today, maybe we 
can discuss writing good mauve tests on the IRC and come up with some 
small set of guidelines.

cheers,
dalibor topic