Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #32897 > unrolled thread

Way to avoid repeating to catch the same exception in overridden method

Back to article view | Back to comp.lang.java.programmer

Contents

  Way to avoid repeating to catch the same exception in overridden method mike <mikaelpetterson@hotmail.com> - 2015-10-19 01:02 -0700
    Re: Way to avoid repeating to catch the same exception in overridden method Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-10-19 12:11 -0400
      Re: Way to avoid repeating to catch the same exception in overridden method Robert Klemme <shortcutter@googlemail.com> - 2015-10-23 08:59 +0200
    Re: Way to avoid repeating to catch the same exception in overridden method Martin Gregorie <martin@address-in-sig.invalid> - 2015-10-19 12:26 +0000
    Re: Way to avoid repeating to catch the same exception in overridden method Silvio <silvio@internet.com> - 2015-10-23 11:00 +0200
      Re: Way to avoid repeating to catch the same exception in overridden method Martin Gregorie <martin@address-in-sig.invalid> - 2015-10-23 10:32 +0000

#32897 — Way to avoid repeating to catch the same exception in overridden method

Hi,


We have the current implementation in our overridden toString() method:

public String toString() {
        return "firstName=" + res.getFirstName() + "{" +
                "lastName=" + res.getLastName() + ", " +
                "address=" + res.getAddress() + "/" + "***" + ", " +
                "zipCode=" + resource.getZipCode() + "}";
    }



Each get can throw an IllegalArgumentExcepion so we need to replace the value with an empty String if it happens( to avoid that our user in this case should see an IllegalArgumentException ).

My question is if there is a more efficient way to override toString() than:

String firstName;
String lastName;
try {
firstName = res.getFirstName();

}catch(IllegalArgumentException e){
firstName = "";
}

try{

lastName = res.getLastName();
}catch(IllegalArgumentException e2){
lastName = "";
}

......

return "firstName=" + res.getFirstName() + "{" +
                "lastName=" + res.getLastName() + ", " +
                "address=" + res.getAddress() + "/" + "***" + ", " +
                "zipCode=" + resource.getZipCode() + "}";


//mike

[toc] | [next] | [standalone]

#32899

On 10/19/2015 4:02 AM, mike wrote:
> Hi,
>
>
> We have the current implementation in our overridden toString() method:
>
> public String toString() {
>          return "firstName=" + res.getFirstName() + "{" +
>                  "lastName=" + res.getLastName() + ", " +
>                  "address=" + res.getAddress() + "/" + "***" + ", " +
>                  "zipCode=" + resource.getZipCode() + "}";
>      }
>
>
>
> Each get can throw an IllegalArgumentExcepion so we need to replace the value with an empty String if it happens( to avoid that our user in this case should see an IllegalArgumentException ).
>
> My question is if there is a more efficient way to override toString() than:
>
> String firstName;
> String lastName;
> try {
> firstName = res.getFirstName();
>
> }catch(IllegalArgumentException e){
> firstName = "";
> }
>
> try{
>
> lastName = res.getLastName();
> }catch(IllegalArgumentException e2){
> lastName = "";
> }
>
> ......
>
> return "firstName=" + res.getFirstName() + "{" +
>                  "lastName=" + res.getLastName() + ", " +
>                  "address=" + res.getAddress() + "/" + "***" + ", " +
>                  "zipCode=" + resource.getZipCode() + "}";

     This won't be "inefficient," since try/catch itself incurs very
little overhead: It's the building and throwing of the exceptions that
chews up resources, and if the various getXxx() calls are going to
build and throw them anyhow, rearranging the try/catch blocks won't
save much.  Besides, I don't think you *can* rearrange them to get
the effect you want, since the "recovery action" for each exception
differs depending on which method threw it.

     You could, I guess, write a little wrapper that takes a String-
yielding lambda expression, evaluates it, and either returns its
value or catches the exception and returns "" -- but if you're
trying to avoid clutter, that sounds like a step in the wrong
direction.

     My big question, though, is about the exception itself.  Why
in the name of all that's holy does a no-argument method throw an
exception that complains about an illegal argument?  Which one of
its non-existent arguments is it complaining about? Does the
complaint actually come from a lower-level method?  For example,
maybe getFirstName() looks like

	String getFirstName() {
	    return getAttribute("FIRST_NAME");
	}

... and it's really getAttribute() that complains.  If that's the
case, I'd suggest a small rewrite like

	String getFirstName() {
	    return getAttributeOrBlank("FIRST_NAME");
	}

	private String getAttributeOrBlank(String attr) {
	    try {
	        return getAttribute(attr);
	    } catch (IllegalArgumentException ex) {
	        return "";
	    }
	}

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#32928

On 19.10.2015 18:11, Eric Sosman wrote:

>      My big question, though, is about the exception itself.  Why
> in the name of all that's holy does a no-argument method throw an
> exception that complains about an illegal argument?  Which one of
> its non-existent arguments is it complaining about?

And even if that exception was legitimate, since it's a 
RuntimeException, which typically indicates a programmer error, it is 
questionable to catch it here.  Of course that can only be decided with 
more context but it is at least worthwhile to ponder.

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

[toc] | [prev] | [next] | [standalone]

#32900

On Mon, 19 Oct 2015 01:02:07 -0700, mike wrote:

> Hi,
> 
> 
> We have the current implementation in our overridden toString() method:
> 
> public String toString() {
>         return "firstName=" + res.getFirstName() + "{" +
>                 "lastName=" + res.getLastName() + ", " +
>                 "address=" + res.getAddress() + "/" + "***" + ", " +
>                 "zipCode=" + resource.getZipCode() + "}";
>     }
> 
> 
> 
> Each get can throw an IllegalArgumentExcepion so we need to replace the
> value with an empty String if it happens( to avoid that our user in this
> case should see an IllegalArgumentException ).
> 
> My question is if there is a more efficient way to override toString()
> than:
>
I'd probably use a StringBuilder: 

public String toString() {
 StringBuilder sb = new StringBuilder();
 sb.append("firstName=");
 sb.append((res.getFirstName() == null ? "" : res.getFirstName()); 
 sb.append("{lastName=");
 sb.append((res.getLastName() == null  ? "" : res.getLastName()));
 sb.append(", address=");
 sb.append((res.getAddress() == null  ? "" : res.getAddress());
 sb.append("/***, zipCode="));
 sb.append((resource.getZipCode() == null  ? "" : resource.getZipCode());
 sb.append("}")
 return sb.toString();
}

or a String.format() method:

public String toString() {
 String s = 
  s.format("firstName=%s{lastName=%s, address="%s/***, zipCode=%s}",
            (res.getFirstName() == null ? "" : res.getFirstName(), 
            (res.getLastName() == null  ? "" : res.getLastName(),
            (res.getAddress() == null  ? "" : res.getAddress(),
            (resource.getZipCode() == null  ? "" : resource.getZipCode());
 return s;
}

both using the conditional operator to avoid try..catch constructs.

These may not be more efficient, but are certainly easier to read and 
more concise, particularly the String.format() statement.


-- 
martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

[toc] | [prev] | [next] | [standalone]

#32929

As others already stated something is fishy if getXXX() without 
parameters can throw an exception unless you have defined that getting 
XXX when the object does not have an XXX at all is invalid. But in that 
case there should also be something to query that, say a hasXXX() or 
something similar. Returning null to indicate a missing value might 
simply work here as well.

I think it is a bad idea to try to hide the exception from the user by 
filling in empty values. The result will be wrong anyway but neither the 
user nor yourself will be alerted.

At the very least replace the COMPLETE toString result with a value that 
indicates that something is wrong and show that to the user. Also a lot 
simpler to code.

But by all means consider throwing the exception if it actually 
indicates a program error. If it doesn't then consider not throwing the 
exception in the first place.


On 10/19/2015 10:02 AM, mike wrote:
> Hi,
>
>
> We have the current implementation in our overridden toString() method:
>
> public String toString() {
>          return "firstName=" + res.getFirstName() + "{" +
>                  "lastName=" + res.getLastName() + ", " +
>                  "address=" + res.getAddress() + "/" + "***" + ", " +
>                  "zipCode=" + resource.getZipCode() + "}";
>      }
>
>
>
> Each get can throw an IllegalArgumentExcepion so we need to replace the value with an empty String if it happens( to avoid that our user in this case should see an IllegalArgumentException ).
>
> My question is if there is a more efficient way to override toString() than:
>
> String firstName;
> String lastName;
> try {
> firstName = res.getFirstName();
>
> }catch(IllegalArgumentException e){
> firstName = "";
> }
>
> try{
>
> lastName = res.getLastName();
> }catch(IllegalArgumentException e2){
> lastName = "";
> }
>
> ......
>
> return "firstName=" + res.getFirstName() + "{" +
>                  "lastName=" + res.getLastName() + ", " +
>                  "address=" + res.getAddress() + "/" + "***" + ", " +
>                  "zipCode=" + resource.getZipCode() + "}";
>
>
> //mike
>

[toc] | [prev] | [next] | [standalone]

#32930

On Fri, 23 Oct 2015 11:00:33 +0200, Silvio wrote:

> As others already stated something is fishy if getXXX() without
> parameters can throw an exception unless you have defined that getting
> XXX when the object does not have an XXX at all is invalid. But in that
> case there should also be something to query that, say a hasXXX() or
> something similar. Returning null to indicate a missing value might
> simply work here as well.
> 
> I think it is a bad idea to try to hide the exception from the user by
> filling in empty values. The result will be wrong anyway but neither the
> user nor yourself will be alerted.
> 
> At the very least replace the COMPLETE toString result with a value that
> indicates that something is wrong and show that to the user. Also a lot
> simpler to code.
> 
> But by all means consider throwing the exception if it actually
> indicates a program error. If it doesn't then consider not throwing the
> exception in the first place.
> 
> 
+1

Returning null is common in both C and Java under these circumstances and 
is simple enough to check for. All you need to is to update the method-
level comment to say that null can be returned and describe the 
circumstances when this can occur (your project DOES mandate the use 
javadocs, doesn't it?).


-- 
martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.java.programmer

csiph-web