sudeep87 / uimafit

Automatically exported from code.google.com/p/uimafit
0 stars 0 forks source link

move AnnotationRetrieval.get(JCas, Class<T>, int) method to testing.util package #24

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
As the javadoc for this method says - its not generally good practice to use 
this method but it is useful for unit testing.  So, I vote that we move this 
method to where it belongs under the testing.util package.  I would also 
entertain a name change to both the class and the method.  

Original issue reported on code.google.com by pvogren@gmail.com on 18 Jun 2010 at 4:45

GoogleCodeExporter commented 8 years ago
I'm thinking change the name to testing.util.IndexUtil and leave the method 
name as get().

Original comment by pvogren@gmail.com on 18 Jun 2010 at 4:58

GoogleCodeExporter commented 8 years ago
I rewrote the method based on CasUtil.getType() and using forward/backward 
iteration. I still wouldn't recommend it for everyday use. In general the 
JCasUtil.iterate() methods should be better. I think though, it may be a good 
idea to merge it into JCasUtil.

Original comment by richard.eckart on 19 Jun 2010 at 4:38

GoogleCodeExporter commented 8 years ago
I moved the AnnotationRetrieval.get method into JCasUtil and refactored the 
supporting tests as well.  

Richard - these methods look really nice.  thanks!  I have two minor quibbles 
though.  Most of the method names seem like they could be changed simply to 
"get" (e.g. getCoveredAnnotations and getAnnotationIterator) without any loss 
of clarity.  I also don't see the purpose of the "iterate" methods and also 
find the name to be confusing since they return an Iterable and not an 
iterator.  

My second quibble is that I find the use of parameter names like aContainer to 
be very distracting when reading your code.  Would you be willing to change 
parameter names to something a little less bothersome (like "container") in the 
future?  I would be happy to go through the code and change the ones that are 
there now.  

Original comment by pvogren@gmail.com on 30 Jun 2010 at 4:17

GoogleCodeExporter commented 8 years ago
I named them iterate() so they do not take up a lot of space in an extended for 
loop. Also I think of them more like extended control functions then getters. 
The JCasUtils is not a bean - neither do I think of it as a factory - which 
would mean to call them create*. As I said - more like a language extension - 
in conjunction with the static imports I am so fond of ;)

Regarding the second: I have the habit of naming arguments differently from 
member variables, so that I do not have to do a this.lala = lala and that I 
never get confused when I remove an argument and suddenly a member variable 
with the same name takes over without me noticing. I recently use the a* for 
arguments and non-prefixed for members. Before I have _* for members and 
non-prefixed arguments.

Original comment by richard.eckart on 30 Jun 2010 at 6:09

GoogleCodeExporter commented 8 years ago
we have two separate threads going on here that I would like to discuss more 
that are unrelated to the original issue.  I'm going to close this one and open 
two more issues.

Original comment by pvogren@gmail.com on 30 Jun 2010 at 3:27