You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by pe...@apache.org on 2007/09/28 13:13:19 UTC
svn commit: r580302 -
/ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
Author: peterreilly
Date: Fri Sep 28 04:13:19 2007
New Revision: 580302
URL: http://svn.apache.org/viewvc?rev=580302&view=rev
Log:
checkstyle: remove inner assignment
Modified:
ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java?rev=580302&r1=580301&r2=580302&view=diff
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java Fri Sep 28 04:13:19 2007
@@ -130,8 +130,12 @@
public boolean isExists() {
InputStream is = null;
try {
- return isReference() ? ((Resource) getCheckedRef()).isExists()
- : (is = getInputStream()) != null;
+ if (isReference()) {
+ return ((Resource) getCheckedRef()).isExists();
+ } else {
+ is = getInputStream();
+ return is != null;
+ }
} catch (IOException ex) {
return false;
} finally {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r580302 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
Posted by Peter Reilly <pe...@gmail.com>.
On 9/28/07, Alexey Solofnenko <A....@mdl.com> wrote:
> In general, I do not see a reason to replace perfectly readable single
> liners with several lines of code - it just increases code bloat.
We agreed to use the checkstyle config
(src/etc/checkstyle/checkstyle-config) for ant
about 2 years ago.
This does ban inner assignments.
> Anybody with high IQ should be able to read old code as is.
- provided one scans it a number of times.
We write enough so the complicated code can be used for complicated
problems - not simple problems.
I am of the firm belief that code should be as simple as possible (but
not simpler!).
This means small methods, "near" variables, small single purpose classes,
one action per statement, quick exit/fail, all statements at the same
level of abstraction,
handling of exceptions in "control" classes.
> In this case
> the change is warranted, but please move "is" variable and try/finally
> inside "else" - the scoping is wrong now.
The scoping has not changed, but is now more obvious......
I have made the change.
Peter
>
> - Alexey.
>
> peterreilly@apache.org wrote:
> > Author: peterreilly
> > Date: Fri Sep 28 04:13:19 2007
> > New Revision: 580302
> >
> > URL: http://svn.apache.org/viewvc?rev=580302&view=rev
> > Log:
> > checkstyle: remove inner assignment
> >
> > Modified:
> > ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
> >
> > Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
> > URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java?rev=580302&r1=580301&r2=580302&view=diff
> > ==============================================================================
> > --- ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java (original)
> > +++ ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java Fri Sep 28 04:13:19 2007
> > @@ -130,8 +130,12 @@
> > public boolean isExists() {
> > InputStream is = null;
> > try {
> > - return isReference() ? ((Resource) getCheckedRef()).isExists()
> > - : (is = getInputStream()) != null;
> > + if (isReference()) {
> > + return ((Resource) getCheckedRef()).isExists();
> > + } else {
> > + is = getInputStream();
> > + return is != null;
> > + }
> > } catch (IOException ex) {
> > return false;
> > } finally {
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > For additional commands, e-mail: dev-help@ant.apache.org
> >
>
> --
> ------------------------------------------------------------------------
> Alexey N. Solofnenko <http://trelony.cjb.net/>
> Pleasant Hill, CA (GMT-8 usually)
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r580302 - /ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
Posted by Alexey Solofnenko <A....@mdl.com>.
In general, I do not see a reason to replace perfectly readable single
liners with several lines of code - it just increases code bloat.
Anybody with high IQ should be able to read old code as is. In this case
the change is warranted, but please move "is" variable and try/finally
inside "else" - the scoping is wrong now.
- Alexey.
peterreilly@apache.org wrote:
> Author: peterreilly
> Date: Fri Sep 28 04:13:19 2007
> New Revision: 580302
>
> URL: http://svn.apache.org/viewvc?rev=580302&view=rev
> Log:
> checkstyle: remove inner assignment
>
> Modified:
> ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
>
> Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java
> URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java?rev=580302&r1=580301&r2=580302&view=diff
> ==============================================================================
> --- ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java (original)
> +++ ant/core/trunk/src/main/org/apache/tools/ant/types/resources/JavaResource.java Fri Sep 28 04:13:19 2007
> @@ -130,8 +130,12 @@
> public boolean isExists() {
> InputStream is = null;
> try {
> - return isReference() ? ((Resource) getCheckedRef()).isExists()
> - : (is = getInputStream()) != null;
> + if (isReference()) {
> + return ((Resource) getCheckedRef()).isExists();
> + } else {
> + is = getInputStream();
> + return is != null;
> + }
> } catch (IOException ex) {
> return false;
> } finally {
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
--
------------------------------------------------------------------------
Alexey N. Solofnenko <http://trelony.cjb.net/>
Pleasant Hill, CA (GMT-8 usually)