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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]