REPL code review

[Maurizio Cimadamore]

On 19/09/15 22:38, Robert Field wrote:
>
> On 09/11/15 08:25, Maurizio Cimadamore wrote:
>>
>> * MemoryFileManager
>>
>>    - Maybe the code of 'list' could be more optimized/lazy by 
>> returning a compound iterator - i.e. an iterator that keeps scanning 
>> the items in the first iterator and, when finished moves onto the 
>> next iterator.
>
> How's this?  It could be more or less lazy: "it" could be pre-filled, 
> or "stdList" could be called at the last minute.
This looks exactly what I was thinking - thanks.
>
>     @Override
>     public Iterable<JavaFileObject> list(JavaFileManager.Location 
> location,
>             String packageName,
>             Set<JavaFileObject.Kind> kinds,
>             boolean recurse)
>             throws IOException {
>         Iterable<JavaFileObject> stdList = 
> stdFileManager.list(location, packageName, kinds, recurse);
>         if (location==CLASS_PATH && packageName.equals("REPL")) {
>             // if the desired list is for our JShell package, lazily 
> iterate over
>             // first the standard list then any generated classes.
>             return () -> new Iterator<JavaFileObject>() {
>                 boolean stdDone = false;
>                 Iterator<? extends JavaFileObject> it;
>
>                 @Override
>                 public boolean hasNext() {
>                     if (it == null) {
>                         it = stdList.iterator();
>                     }
>                     if (it.hasNext()) {
>                         return true;
>                     }
>                     if (stdDone) {
>                         return false;
>                     } else {
>                         stdDone = true;
>                         it = generatedClasses().iterator();
>                         return it.hasNext();
>                     }
>                 }
>
>                 @Override
>                 public JavaFileObject next() {
>                     if (!hasNext()) {
>                         throw new NoSuchElementException();
>                     }
>                     return it.next();
>                 }
>
>             };
>         } else {
>             return stdList;
>         }
>     }
>
>>
>>    - comments: is it normal to repeat all comments from the 
>> JavaFileManager interface?
>
> I don't think it is normal.
>
> I've found it very useful to have them there both during 
> implementation and maintenance.  If you feel they should be removed, I 
> will.
I believe most of the code I've seen around tends not to repeat the 
documentation, unless something significantly different occurs in the 
method, so that you feel like you need to extend the javadoc in some 
way. But if you feel there's some value in keeping the docs, then again 
I'm not strongly against it - just noting it is mildly unusual.
>
>>
>>    - watch out for unused methods: dumpClasses, findGeneratedClass, 
>> findGeneratedBytes, inferModuleName, listModueLocations (maybe this 
>> stuff is for Jigsaw? If so, maybe add a comment) 
>
> I've added comment (in my local repo).
>
>     // For debugging dumps
> dumpClasses
>
>     // For restoring process-local execution support
> findGeneratedClass, findGeneratedBytes
> and several others
>
>     // Make compatible with Jigsaw
> inferModuleName, listModueLocations
Thanks
Maurizio
>
> Thanks,
> Robert
>