-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark,

I know you posted this a while ago, but I have some comments:

On 5/4/2010 9:10 AM, Mark Shifman wrote:
>       public <T> T getNextElement(String theElement, String elementAfter, 
> Class <T>clazz) {
>               String elname = "";
>               T h = null;
>               try {
>                       while(reader.hasNext()){
>                         if(reader.peek().isStartElement()){
>                                elname = 
> reader.peek().asStartElement().getName().getLocalPart();
>                                if(elname.equals(theElement)){
>                                      h= u.unmarshal(reader, clazz).getValue();
>                                      return h;
>                                }
>                         } else if(reader.peek().isEndElement()){
>                                       elname = 
> reader.peek().asEndElement().getName().getLocalPart();
>                                       if(elname.equals(elementAfter)){
>                                               return h;
>                                       }
>                               }
> 
>                         reader.nextEvent();
>                       }
>               } catch (XMLStreamException e) {
>                       throw new RuntimeException(e);
>               } catch (JAXBException e) {
>                       throw new RuntimeException(e);
>               }
>               return h;
>     }

You could put the "return h" above the exception handlers to make it
clear that the try block must run to completion in order to return a
value. That's just a matter of taste, though.

> It also has a close method to clean up after I have gotten all the elements.
>       public void close(){
>               try {
>                       reader.close();
>               } catch (XMLStreamException e) {
>                       //quietly

Don't do this: at least log the exception to stdout, or you may miss
something important at some point.

>               }
>               IOUtils.closeQuietly(jxb_in);
>               u=null;
>       }

You might want to put the call to IOUtils.closeQuietly into a finally
block: a RuntimeException could skip the call to your "close" method.

Whenever there's a close method available, you should call it.
Whenever you call it, you should probably call it in a finally blocks.

>>>     private InputStream jxb_in;
>>>
>>>     public static JAXBMascot getInstance(InputStream in) {
>>>             JAXBMascot m = new JAXBMascot();
>>>             try {
>>>                     m.setJxb_in(in);
>>>                     
>>> m.setReader(XMLInputFactory.newInstance().createXMLEventReader(in));
>>>             } catch (Exception e) {
>>>                     log.fatal("error getting JAXBMascot instance");
>>>                     IOUtils.closeQuietly(in);
>>>                     throw new RuntimeException(e);
>>>             }

Should you close even if no Exception has been thrown? The above does
not cover Error conditions, for example. Perhaps you wanted to catch
Throwable instead?

- -chris
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvxpQMACgkQ9CaO5/Lv0PCyOgCfabMgxsAJzNplBsspeQnn83i2
KSwAn36FoEeFGJIQppwTpD5Ju338qIVX
=HAkp
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to