uschindler commented on code in PR #13205:
URL: https://github.com/apache/lucene/pull/13205#discussion_r1537285277


##########
lucene/core/src/java/org/apache/lucene/store/IOContext.java:
##########
@@ -62,91 +55,36 @@ public enum Context {
 
   public static final IOContext LOAD = new IOContext(false, true);
 
-  public IOContext() {
-    this(false, false);
-  }
-
-  public IOContext(FlushInfo flushInfo) {
-    assert flushInfo != null;
-    this.context = Context.FLUSH;
-    this.mergeInfo = null;
-    this.readOnce = false;
-    this.load = false;
-    this.flushInfo = flushInfo;
-  }
-
-  public IOContext(Context context) {
-    this(context, null);
+  @SuppressWarnings("incomplete-switch")
+  public IOContext {
+    switch (context) {
+      case MERGE -> Objects.requireNonNull(
+          mergeInfo, "mergeInfo must not be null if context is MERGE");
+      case FLUSH -> Objects.requireNonNull(
+          flushInfo, "flushInfo must not be null if context is FLUSH");
+    }
   }
 
   private IOContext(boolean readOnce, boolean load) {
-    this.context = Context.READ;
-    this.mergeInfo = null;
-    this.readOnce = readOnce;
-    this.load = load;
-    this.flushInfo = null;
-  }
-
-  public IOContext(MergeInfo mergeInfo) {
-    this(Context.MERGE, mergeInfo);
+    this(Context.READ, null, null, readOnce, load);
   }
 
-  private IOContext(Context context, MergeInfo mergeInfo) {
-    assert context != Context.MERGE || mergeInfo != null
-        : "MergeInfo must not be null if context is MERGE";
-    assert context != Context.FLUSH : "Use IOContext(FlushInfo) to create a 
FLUSH IOContext";
-    this.context = context;
-    this.readOnce = false;
-    this.load = false;
-    this.mergeInfo = mergeInfo;
-    this.flushInfo = null;
+  private IOContext(Context context) {
+    this(context, null, null, false, false);
   }
 
-  /**
-   * This constructor is used to initialize a {@link IOContext} instance with 
a new value for the
-   * readOnce variable.
-   *
-   * @param ctxt {@link IOContext} object whose information is used to create 
the new instance
-   *     except the readOnce variable.
-   * @param readOnce The new {@link IOContext} object will use this value for 
readOnce.
-   */
-  public IOContext(IOContext ctxt, boolean readOnce) {
-    this.context = ctxt.context;
-    this.mergeInfo = ctxt.mergeInfo;
-    this.flushInfo = ctxt.flushInfo;
-    this.readOnce = readOnce;
-    this.load = false;
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(context, flushInfo, mergeInfo, readOnce, load);
+  /** Creates an IOContext for flushing. */
+  public IOContext(FlushInfo flushInfo) {
+    this(Context.FLUSH, null, flushInfo, false, false);
   }
 
-  @Override
-  public boolean equals(Object obj) {
-    if (this == obj) return true;
-    if (obj == null) return false;
-    if (getClass() != obj.getClass()) return false;
-    IOContext other = (IOContext) obj;
-    if (context != other.context) return false;
-    if (!Objects.equals(flushInfo, other.flushInfo)) return false;
-    if (!Objects.equals(mergeInfo, other.mergeInfo)) return false;
-    if (readOnce != other.readOnce) return false;
-    if (load != other.load) return false;
-    return true;
+  /** Creates an IOContext for merging. */
+  public IOContext(MergeInfo mergeInfo) {
+    this(Context.MERGE, mergeInfo, null, false, false);
   }
 
-  @Override
-  public String toString() {
-    return "IOContext [context="
-        + context
-        + ", mergeInfo="
-        + mergeInfo
-        + ", flushInfo="
-        + flushInfo
-        + ", readOnce="
-        + readOnce
-        + "]";
+  /** Return a copy of this IOContext with {@link #readOnce} set to {@code 
true}. */
+  public IOContext toReadOnce() {
+    return new IOContext(context, mergeInfo, flushInfo, true, load);

Review Comment:
   I missed to mention this in the issue description: This seems to be a bug in 
current code, because the copy-ctor did not do what its documentation said.
   
   The code is only used in 
[VariableGapTermsIndexReader.java](https://github.com/apache/lucene/pull/13205/files#diff-6cc5c40198a0a7cb2746909210a4ad045cb71a970c4d7fa023dbb02079eb33c7)
 at the moment. I don't know if ever `load=true` there, so changing this here 
as a bugfix is possibly a noop.
   
   Unless theres a real reason to enforce this, we must document it. What do 
you think?



-- 
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...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to