You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Daniel Mueller <da...@digitalstrider.com> on 2007/06/10 21:10:50 UTC
Ideas for code clarity
Hi all,
While scouting through the source code I stumbled across
org.apache.http.conn.EofSensorInputStream. You have there three methods
along the lines of:
protected void checkEOF(int eof) throws IOException {
if ((wrappedStream != null) && (eof < 0)) {
try {
boolean scws = true; // should close wrapped stream?
if (eofWatcher != null)
scws = eofWatcher.eofDetected(wrappedStream);
if (scws)
wrappedStream.close();
} finally {
wrappedStream = null;
}
}
}
This could be simplified by introducing:
private static final EofSensorWatcher NULL_EOF_WATCHER = new
EofSensorWatcher() {
public boolean eofDetected(InputStream wrapped) throws IOException {
return true;
}
public boolean streamAbort(InputStream wrapped) throws IOException {
return true;
}
public boolean streamClosed(InputStream wrapped) throws IOException
{
return true;
}
};
which can be assigned in the constructor if there's no default
EofSensorWatcher submitted. This changes the implementation to the
following:
protected void checkEOF(int eof) throws IOException {
if ((wrappedStream != null) && (eof < 0)) {
try {
if (eofWatcher.eofDetected(wrappedStream))
wrappedStream.close();
} finally {
wrappedStream = null;
}
}
}
which I think reads itself nicer. I avoid null checks in code when I can. I
like NULL implementations much more.
Just some thoughts.
Cheers,
Daniel
Re: Ideas for code clarity
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2007-06-10 at 21:10 +0200, Daniel Mueller wrote:
> Hi all,
>
> While scouting through the source code I stumbled across
> org.apache.http.conn.EofSensorInputStream. You have there three methods
> along the lines of:
>
> protected void checkEOF(int eof) throws IOException {
>
> if ((wrappedStream != null) && (eof < 0)) {
> try {
> boolean scws = true; // should close wrapped stream?
> if (eofWatcher != null)
> scws = eofWatcher.eofDetected(wrappedStream);
> if (scws)
> wrappedStream.close();
> } finally {
> wrappedStream = null;
> }
> }
> }
>
> This could be simplified by introducing:
>
> private static final EofSensorWatcher NULL_EOF_WATCHER = new
> EofSensorWatcher() {
> public boolean eofDetected(InputStream wrapped) throws IOException {
> return true;
> }
>
> public boolean streamAbort(InputStream wrapped) throws IOException {
> return true;
> }
>
> public boolean streamClosed(InputStream wrapped) throws IOException
> {
> return true;
> }
> };
>
> which can be assigned in the constructor if there's no default
> EofSensorWatcher submitted. This changes the implementation to the
> following:
>
> protected void checkEOF(int eof) throws IOException {
>
> if ((wrappedStream != null) && (eof < 0)) {
> try {
> if (eofWatcher.eofDetected(wrappedStream))
> wrappedStream.close();
> } finally {
> wrappedStream = null;
> }
> }
> }
>
> which I think reads itself nicer. I avoid null checks in code when I can. I
> like NULL implementations much more.
>
> Just some thoughts.
>
> Cheers,
> Daniel
Hi Daniel,
The best way to go about things like that is to open a JIRA ticket and
attach a patch to it. Someone will eventually pick up the report and
will get back to you with some feedback. This particular piece of code
was initially written by Roland, so he should take a look at the
proposed changes.
Cheers
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: Ideas for code clarity
Posted by Roland Weber <os...@dubioso.net>.
Hi Daniel,
>> You have to cross-reference with either the
>> JavaDocs in the interface or the actual call
>> to understand what "true" or "false" means.
>> Prettier code is not always more readable.
>
> It's always interesting to get different views on a programming subject,
> especially if the other developer is used to different programming habits.
> Since I'm using IDE's or enhanced editors, the javadoc is a mouse movement
> or a keypress away, and I can jump to implementing methods also very
> easily.
> So I guess this influences my view of code readability.
Ha! That supports my argument that IDEs encourage sloppyness.
(But so do safety belts, or helmets... ;-)
> It's your decision what to do with this one. I'll provide the patch if
> you're interested.
Give it a try!
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org
Re: Ideas for code clarity
Posted by Daniel Mueller <da...@gmail.com>.
Hi Roland
you implement an additional class and perform
> unnecessary method calls to avoid null checks?
> I must be getting old ;-)
Lol. Valid point though on the additional method calls. I'm simply used to
write the code with maximum readability quite high on my priority list. It
might not apply here.
The overhead in this particular case is not a
> problem, so I would agree to apply the patch if
> you provide one. However, I have to insist on
> additional comments in your inner class, like:
>
> return true; // close the stream
certainly no problem.
In the current code, the line that assigns a
> default to "scws" explains itself, or can be
> understood by skipping two lines down. In your
> suggestion, "return true" carries no meaning.
> You have to cross-reference with either the
> JavaDocs in the interface or the actual call
> to understand what "true" or "false" means.
> Prettier code is not always more readable.
It's always interesting to get different views on a programming subject,
especially if the other developer is used to different programming habits.
Since I'm using IDE's or enhanced editors, the javadoc is a mouse movement
or a keypress away, and I can jump to implementing methods also very easily.
So I guess this influences my view of code readability.
It's your decision what to do with this one. I'll provide the patch if
you're interested.
Cheers,
Daniel
Re: Ideas for code clarity
Posted by Roland Weber <os...@dubioso.net>.
Hello Daniel,
you implement an additional class and perform
unnecessary method calls to avoid null checks?
I must be getting old ;-)
The overhead in this particular case is not a
problem, so I would agree to apply the patch if
you provide one. However, I have to insist on
additional comments in your inner class, like:
return true; // close the stream
In the current code, the line that assigns a
default to "scws" explains itself, or can be
understood by skipping two lines down. In your
suggestion, "return true" carries no meaning.
You have to cross-reference with either the
JavaDocs in the interface or the actual call
to understand what "true" or "false" means.
Prettier code is not always more readable.
cheers,
Roland
---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org