jonfreedman commented on pull request #32: URL: https://github.com/apache/commons-io/pull/32#issuecomment-913855307
I have a feeling I just lifted all the java doc from the underlying methods, they may have changed in the last 5 years and I'll also incorporate your suggestions. Regarding the constructors/factory methods, personally I would prefer to replace the whole lot with a builder but that's obviously quite a departure from the existing code, I had designed this PR to be as close as possible to the existing but I'm happy to make it 'better' if it would be accepted? ---- On Mon, 06 Sep 2021 19:40:47 +0100 ***@***.*** wrote ---- @garydgregory requested changes on this pull request. @jonfreedman Thank you for the update. Hi, I did a review and scattered my comments throughout. In src/main/java/org/apache/commons/io/input/Tailer.java: > + * @return <code>true</code> if and only if the tailable exists; + * <code>false</code> otherwise + */ + boolean exists(); + + /** + * Tests if this tailable is newer than the specified time reference. + * + * @param timeMillis the time reference measured in milliseconds since the + * epoch (00:00:00 GMT, January 1, 1970). + * @return true if this tailable has been modified after the given time reference. + */ + boolean isFileNewer(final long timeMillis); + + /** + * @param mode the access mode ***@***.*** RandomAccessFile} The 1st sentence is missing for this method. In src/main/java/org/apache/commons/io/input/Tailer.java: > @@ -556,4 +784,181 @@ private long readLines(final RandomAccessFile reader) throws IOException { return rePos; } } + + /** + * Abstraction on ***@***.*** java.io.File} which allows substitution of remote files for example using jCIFS + * + * @since 2.12.0 + */ + public interface Tailable { + /** + * @return The name of the file denoted by this tailable The 1st sentence is missing for this method. In src/main/java/org/apache/commons/io/input/Tailer.java: > + /** + * @param mode the access mode ***@***.*** RandomAccessFile} + * @return a random access file stream to read from + * @throws FileNotFoundException if the tailable object does not exist + */ + RandomAccessTailable getRandomAccess(final String mode) throws FileNotFoundException; + } + + /** + * Abstraction on ***@***.*** java.io.RandomAccessFile} which allows substitution of remote files for example using jCIFS + * + * @since 2.12.0 + */ + public interface RandomAccessTailable extends Closeable { + /** + * Returns the current offset in this tailable. "Returns" -> "Gets" In src/main/java/org/apache/commons/io/input/Tailer.java: > + * @exception IOException if an I/O error occurs. + */ + long getFilePointer() throws IOException; + + /** + * Sets the file-pointer offset, measured from the beginning of this + * tailable, at which the next read or write occurs. The offset may be + * set beyond the end of the tailable. Setting the offset beyond the end + * of the tailable does not change the tailable length. The tailable + * length will change only by writing after the offset has been set beyond + * the end of the tailable. + * + * @param pos the offset position, measured in bytes from the + * beginning of the tailable, at which to set the + * tailable pointer. + * @exception IOException if ***@***.*** pos} is less than ***@***.***" -> ***@***.***" In src/main/java/org/apache/commons/io/input/Tailer.java: > + */ + public interface Tailable { + /** + * @return The name of the file denoted by this tailable + */ + String getName(); + + /** + * @return The full path of this tailable + */ + String getPath(); + + /** + * Returns the length of this tailable + * + * @return The length, in bytes, of this tailable, or <code>0L</code> foo -> ***@***.*** foo} In src/main/java/org/apache/commons/io/input/Tailer.java: > + * @param timeMillis the time reference measured in milliseconds since the + * epoch (00:00:00 GMT, January 1, 1970). + * @return true if this tailable has been modified after the given time reference. + */ + boolean isFileNewer(final long timeMillis); + + /** + * @param mode the access mode ***@***.*** RandomAccessFile} + * @return a random access file stream to read from + * @throws FileNotFoundException if the tailable object does not exist + */ + RandomAccessTailable getRandomAccess(final String mode) throws FileNotFoundException; + } + + /** + * Abstraction on ***@***.*** java.io.RandomAccessFile} which allows substitution of remote files for example using jCIFS Sentences should end in a period. In src/main/java/org/apache/commons/io/input/Tailer.java: > + + /** + * Abstraction on ***@***.*** java.io.File} which allows substitution of remote files for example using jCIFS + * + * @since 2.12.0 + */ + public interface Tailable { + /** + * @return The name of the file denoted by this tailable + */ + String getName(); + + /** + * @return The full path of this tailable + */ + String getPath(); Confusing API name because it does not return a Path; either rename the API or make it return a Path. In src/main/java/org/apache/commons/io/input/Tailer.java: > @@ -556,4 +784,181 @@ private long readLines(final RandomAccessFile reader) throws IOException { return rePos; } } + + /** + * Abstraction on ***@***.*** java.io.File} which allows substitution of remote files for example using jCIFS + * + * @since 2.12.0 + */ + public interface Tailable { + /** + * @return The name of the file denoted by this tailable + */ + String getName(); The name of... what? the base file name? The full file name? In src/main/java/org/apache/commons/io/input/Tailer.java: > @@ -191,6 +203,18 @@ public Tailer(final File file, final TailerListener listener, final long delayMi this(file, listener, delayMillis, false); } + /** There are way too many new factory APIs here IMO. For the first cut, let's add one and see what other use-cases require in the future. If a delay is provided, please make that a Duration, not a long. I'll redo the internals after this PR comes in to use a Duration instead of a long. If you add a constructor, make it private or package private. —You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org