A View over java.lang.String - improved take II

Question

(This post elaborates on A string view over a Java String.)

This time, I have incorporated both the great answers in the previous iteration:

• https://codereview.stackexchange.com/a/293506/58360 by Alexander Ivanchenko,
• https://codereview.stackexchange.com/a/293489/58360 by Toby Speight.

Now my code looks like follows:

com.github.coderodde.util.StringView.java:

package com.github.coderodde.util;
import java.util.Objects;
import java.util.stream.IntStream;
/**
 * This class implements a string view that can be enlarged, shrinked or
 * shifted.
 * 
 * @version 1.1.0 (Aug 29, 2024)
 * @since 1.1.0 (Aug 29, 2024)
 */
public final class StringView implements CharSequence {
 private final String string;
 private final int viewOffset;
 private final int viewLength;
 
 /**
 * Constructs this string view.
 * 
 * @param string the owner string.
 * @param viewOffset the offset in the owner string. 
 * @param viewLength the length of the view.
 */
 public StringView(final String string,
 final int viewOffset,
 final int viewLength) {
 
 this.string = 
 Objects.requireNonNull(string, "The input string is null.");
 
 checkOnConstruction(viewOffset, 
 viewLength);
 
 this.viewOffset = viewOffset;
 this.viewLength = viewLength;
 }
 
 /**
 * Constructs this string view. The resulting view will cover the entire 
 * owner string.
 * 
 * @param string the owner string.
 */
 public StringView(final String string) {
 this(string, 0, string.length());
 }
 
 /**
 * Accesses the {@code index}th character of this string view.
 * 
 * @param index the index of the desired character.
 * 
 * @return the {@code index}th character of this string view.
 */
 public char charAt(final int index) {
 checkIndex(index);
 return string.charAt(viewOffset + index);
 }
 
 /**
 * Return a {@link java.lang.String} holding the contents of this string
 * view.
 * 
 * @return the string representation of this string view.
 */
 @Override
 public String toString() {
 return string.substring(viewOffset, viewOffset + viewLength);
 }
 @Override
 public int hashCode() {
 int hash = 7;
 hash = 97 * hash + hashCodeImpl();
 hash = 97 * hash + viewOffset;
 hash = 97 * hash + viewLength;
 return hash;
 }
 
 @Override
 public boolean equals(Object obj) {
 if (this == obj) {
 return true;
 }
 
 if (obj == null) {
 return false;
 }
 
 if (getClass() != obj.getClass()) {
 return false;
 }
 
 final StringView other = (StringView) obj;
 
 if (viewOffset != other.viewOffset) {
 return false;
 }
 
 if (viewLength != other.viewLength) {
 return false;
 }
 
 return Objects.equals(string, other.string);
 }
 
 public String getOwnerString() {
 return string;
 }
 
 public int getViewOffset() {
 return viewOffset;
 }
 
 public int getViewLength() {
 return viewLength;
 }
 
 /**
 * Attempts to shift the view by {@code steps} steps to the left.
 * 
 * @param steps the number of steps to shift.
 * 
 * @return a new string view with the shifted contents.
 */
 public StringView shiftLeft(final int steps) {
 checkStepsNotBelowZero(steps);
 
 final int newViewOffset = Math.max(0, viewOffset - steps);
 
 return new StringView(string, newViewOffset, viewLength);
 }
 
 /**
 * Attempts to shift the view by {@code steps} steps to the right.
 * 
 * @param steps the number of steps to shift.
 * 
 * @return a new string view with the shifted contents.
 */
 public StringView shiftRight(final int steps) {
 checkStepsNotBelowZero(steps);
 
 final int newViewOffset = Math.min(string.length(), viewOffset + steps);
 
 return new StringView(string, newViewOffset, viewLength);
 }
 
 /**
 * Shift either to left or right by {@code steps} steps. If the argument is
 * negative, shifts to the left. Otherwise, shifts to the right.
 * 
 * @param steps the number of steps to shift.
 * 
 * @return a new string view with the shifted contents.
 */
 public StringView shift(final int steps) {
 if (steps < 0) {
 return shiftLeft(-steps);
 } else {
 return shiftRight(steps);
 }
 }
 
 /**
 * Shrinks this string view by {@code length} elements from the tail of the
 * string view.
 * 
 * @param length the length to shrinky by.
 * 
 * @return the shrinked string view.
 */
 public StringView shrink(final int length) {
 checkLengthNotNegative(length);
 checkLengthIsNotLargerThanCurrentViewLength(length);
 return new StringView(string, viewOffset, viewLength - length);
 }
 
 /**
 * Grows this string view by {@code length} elements towards the tail of the
 * string view.
 * 
 * @param length the length to grow by.
 * 
 * @return the grown string view.
 */
 public StringView grow(final int length) {
 checkLengthNotNegative(length);
 checkViewDoesNotOutgrow(length);
 return new StringView(string, viewOffset, viewLength + length);
 }
 @Override
 public int length() {
 return viewLength;
 }
 @Override
 public boolean isEmpty() {
 return viewLength == 0;
 }
 @Override
 public CharSequence subSequence(final int start, 
 final int end) {
 
 final int subSequenceLength = end - start;
 
 if (subSequenceLength > viewLength) {
 throw new IllegalArgumentException(
 String.format(
 "Requested sequence has %d character(s). " + 
 "This view has %d.", 
 subSequenceLength,
 viewLength));
 }
 
 if (start + subSequenceLength > viewOffset + viewLength) {
 throw new IllegalArgumentException(
 String.format(
 "Overflow on the right by %d character(s).", 
 start + subSequenceLength 
 - viewOffset 
 - viewLength));
 }
 
 return new StringView(string, 
 viewOffset + start, 
 subSequenceLength);
 }
 @Override
 public IntStream chars() {
 return CharSequence.super.chars(); 
 }
 @Override
 public IntStream codePoints() {
 return CharSequence.super.codePoints(); 
 }
 
 private int hashCodeImpl() {
 int hash = 13;
 
 for (int i = viewOffset; i < viewOffset + viewLength; i++) {
 hash = 71 * hash + Character.hashCode(string.charAt(i));
 }
 
 return hash;
 }
 
 private void checkIndex(final int index) {
 if (index < 0) {
 final String exceptionMessage = 
 String.format("index (%d) < 0", index);
 
 throw new IndexOutOfBoundsException(exceptionMessage);
 }
 
 if (index >= viewLength) {
 final String exceptionMessage = 
 String.format(
 "index (%d) >= viewLength (%d)", 
 index, 
 viewLength);
 
 throw new IndexOutOfBoundsException(exceptionMessage);
 }
 }
 
 private void checkViewDoesNotOutgrow(final int length) {
 if (viewOffset + length > string.length()) {
 final String exceptionMessage = 
 String.format(
 "New view outgrows the string view parent by %d characters.",
 viewOffset + length - string.length());
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 }
 
 private void checkLengthNotNegative(final int length) {
 if (length < 0) {
 final String exceptionMessage = 
 String.format("length (%d) < 0", length);
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 }
 
 private void checkLengthIsNotLargerThanCurrentViewLength(final int length) {
 if (length > viewLength) {
 final String exceptionMessage = 
 String.format(
 "length (%d) > viewLength (%d)",
 length, 
 viewLength);
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 }
 
 private void checkStepsNotBelowZero(final int steps) {
 if (steps < 0) {
 final String exceptionMessage = 
 String.format("steps (%d) < 0", steps);
 throw new IllegalArgumentException(exceptionMessage);
 }
 }
 
 private void checkOnConstruction(final int viewOffset,
 final int viewLength) {
 
 if (viewOffset < 0) {
 final String exceptionMessage = 
 String.format(
 "offset (%d) < 0",
 viewOffset);
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 
 if (viewOffset > string.length()) {
 final String exceptionMessage = 
 String.format(
 "offset (%d) > string.length (%d)",
 viewOffset,
 string.length());
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 
 if (viewLength < 0) {
 final String exceptionMessage = 
 String.format(
 "viewLength (%d) < 0", 
 viewLength);
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 
 if (viewLength > string.length()) {
 final String exceptionMessage = 
 String.format(
 "viewLength (%d) > string.length (%d)",
 viewLength,
 string.length());
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 
 if (viewOffset + viewLength > string.length()) {
 final String exceptionMessage = 
 String.format(
 "View outside the right border by %d characters.",
 viewOffset + viewLength - string.length());
 
 throw new IllegalArgumentException(exceptionMessage);
 }
 }
}

com.github.coderodde.util.StringViewTest.java:

package com.github.coderodde.util;
import java.util.PrimitiveIterator.OfInt;
import java.util.stream.IntStream;
import org.junit.Test;
import static org.junit.Assert.*;
public class StringViewTest {
 
 @Test
 public void testCharAt() {
 StringView view = new StringView("abcd");
 
 assertEquals('a', view.charAt(0));
 assertEquals('b', view.charAt(1));
 assertEquals('c', view.charAt(2));
 assertEquals('d', view.charAt(3));
 
 view = view.shrink(1);
 view = view.shift(1);
 
 assertEquals('b', view.charAt(0));
 assertEquals('c', view.charAt(1));
 assertEquals('d', view.charAt(2));
 }
 
 @Test
 public void testToString() {
 StringView view = new StringView("abcde");
 view = view.shrink(2);
 view = view.shiftRight(1);
 
 final String s = view.toString();
 assertEquals("bcd", s);
 }
 
 @Test
 public void testShiftLeft() {
 StringView view = new StringView("0123456789",3, 3);
 view = view.shiftLeft(2);
 
 assertEquals("123", view.toString());
 }
 
 @Test
 public void testShiftRight() {
 StringView view = new StringView("0123456", 3, 2);
 view = view.shiftRight(1);
 
 assertEquals("45", view.toString());
 }
 @Test
 public void testShift() {
 StringView view = new StringView("12345", 1, 3);
 
 view = view.shift(-1);
 
 assertEquals("123", view.toString());
 
 view = view.shift(2);
 
 assertEquals("345", view.toString());
 }
 @Test
 public void testShrink() {
 StringView view = new StringView("abcde12345", 0, 6);
 
 view = view.shiftRight(2);
 view = view.shrink(4);
 
 assertEquals("cd", view.toString());
 }
 @Test
 public void testGrow() {
 StringView view = new StringView("abcde12345", 2, 4);
 
 view = view.grow(2);
 
 assertEquals("cde123", view.toString());
 }
 
 @Test
 public void intStreamChars() {
 final StringView view = new StringView("abcdef", 1, 4);
 final IntStream stream = view.chars();
 
 final OfInt ofInt = stream.iterator();
 
 assertEquals((Integer)("b".codePointAt(0)), ofInt.next());
 assertEquals((Integer)("c".codePointAt(0)), ofInt.next());
 assertEquals((Integer)("d".codePointAt(0)), ofInt.next());
 assertEquals((Integer)("e".codePointAt(0)), ofInt.next());
 }
 
 @Test
 public void intStreamCodePoints() {
 final StringView view = new StringView("abcdef", 1, 4);
 final IntStream stream = view.codePoints();
 
 final OfInt ofInt = stream.iterator();
 
 assertEquals((Integer)("b".codePointAt(0)), ofInt.next());
 assertEquals((Integer)("c".codePointAt(0)), ofInt.next());
 assertEquals((Integer)("d".codePointAt(0)), ofInt.next());
 assertEquals((Integer)("e".codePointAt(0)), ofInt.next());
 }
 
 @Test
 public void substring() {
 final StringView view = new StringView("123456", 1, 4);
 final CharSequence seq = view.subSequence(1, 4);
 
 assertEquals("345", seq.toString());
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void substringThrowsOnToLong() {
 final StringView view = new StringView("123456", 1, 4);
 view.subSequence(2, 6);
 }
 
 @Test(expected = IndexOutOfBoundsException.class)
 public void throwsOnNegativeIndex() {
 new StringView("abc").charAt(-1);
 }
 
 @Test(expected = IndexOutOfBoundsException.class)
 public void throwsOnTooLargeIndex() {
 new StringView("abc").charAt(3);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnOutGrow() {
 StringView view = new StringView("abcd", 1, 2);
 view.grow(2);
 }
 
 @Test(expected = IllegalArgumentException.class) 
 public void throwsOnNegativeLength() {
 new StringView("abc").shrink(-1);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnShrinkWhenRequestedLengthSmallerThanViewLength() {
 new StringView("12345", 1, 3).shrink(4);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnBelowZeroStepsLeft() {
 new StringView("12345", 1, 3).shiftLeft(-1);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnBelowZeroStepsRight() {
 new StringView("12345", 1, 3).shiftRight(-1);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnSubSequenceReqestNotLargerThanOriginal() {
 new StringView("12345").subSequence(1, 7);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnSubSequenceOutGrowsParentString() {
 new StringView("12345").subSequence(3, 6);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnConstructionWhenNegativeViewOffset() {
 new StringView("abc", -1, 2);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnConstructionWhenViewOffsetOutsideOfString() {
 new StringView("abc", 4, 2);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnConstructionWhenViewLengthNegative() {
 new StringView("abc", 4, -1);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnConstructionWhenViewLengthLargerThanString() {
 new StringView("abc", 4, 4);
 }
 
 @Test(expected = IllegalArgumentException.class)
 public void throwsOnConstructionWhenViewOutgrowsString() {
 new StringView("abcd1234", 4, 5);
 }
}

Critique request

Please, feel free to give any constructive commentary.

• OrangeDog: But why? Strings are immutable. Your JRE is free to make the optimisation of not copying the buffer when creating a substring.
• TorbenPutkonen: @OrangeDog The JRE does not make that optimization. They used to, but it lead to memory leaks so the feature was removed.

Answer 1

grow / shrink / shift

• There's no need to construct a new StringView instance if any of these methods receives zero as an argument.

Add guard clauses returning this instance. And it will be a good idea to update the test suite to verify that all of these methods return the same instance when they are called with zero.

• Parameter names

The names given to method parameters of grow / shrink / shift are misleading.

Let's take a look at grow() as an example:

/**
 * Grows this string view by {@code length} elements towards the tail of the
 * string view.
 *
 * @param length the length to grow by.
 *
 * @return the grown string view.
 */
public StringView grow(final int length) {
 checkLengthNotNegative(length);
 checkViewDoesNotOutgrow(length);
 return new StringView(string, viewOffset, viewLength + length);
}

The parameter name length doesn't effectively communicate what value it's meant to represent. It's very deceptive and creates an impression that this method expect a new length of the view.

To avoid ambiguity, consider using the parameter names like:

growBy, shrinkBy, shiftBy.

length()

It doesn't seem StringView type can benefit from defining an additional method reporting its length.

You can ditch getViewLength().

^{Sometimes, there are cases when a type requires more than one method responsible for doing the same thing in order to conform to several interfaces. But that's not we have here.}

equals / hashCode

Presented implementation of equals() can be improved in terms of readability and style.

But first, let's discuss its logic.

Consider these two strings:

var first = "same_thing <-- ";
var second = " --> same_thing";

According to your implementation of the equals / hashCode contract, new StringView(first, 0, 10) and new StringView(second, 5, 10) are not equal, despite both of them describing the "same_thing" substring (pun intended).

^{Since your implementation mandates that the same underlying strings should be equal, and the offset and length identical}

In the first question about StringView you said that it's meant to represent the concept of a substring. If so, it doesn't play well with the current definition of the StringView equality, when two views with the same content are not necessarily equal.

If that wasn't your intention, and views over the first and the second string from the example above should be equal, then here are the changes you need to make:

• replace checking equality of the underlying strings with equality check of substrings each view describes;

• there should be no mention of offsets neither in equals(), nor in hashCode() (it's important to ensure that instances that are equal produce the same hash, otherwise it'll be a violation of the equals / hashCode contract).

Regarding stile, here's how equals() can be rewritten using Java 16 Pattern Matching for instanceof:

@Override
public boolean equals(Object o) {
 return o instanceof StringView other
 && viewLength == other.viewLength
 && contentEquals(other);
}

^{Where contentEquals() compares the actual content of the two views}

Answer 2

There are only two hard things in computer science: concurrency and getting programmers to pay attention to naming things.

Naming is really simple. You look at what the component you are naming does, and give it a name that describes it's purpose. Then you look at the name and if it describes more than one thing, you make a more descriptive name. If a couple of iterations doesn't help, then the component may be violating well known programming practises.

private final String string;

"String" describes every string in the universe. Should be originalString.

checkOnConstruction(viewOffset, viewLength);

This name describes the time it is executed, not what it does and why. Could be validateConstructorParameters. But then you're only validating two of the three parameters. Maybe it is not worth having these checks in a separate method?

private void checkStepsNotBelowZero(final int steps) {

This makes sense only if you know the context it is called from and steps does not exactly describe what the value represents. The way you have implemented the shift method input validation (e.g. relying partly on the constructor) produces error messages that are not directly connected to the actual invalid input values. This method should be renamed validateShiftOffset(final int offset) and it should cover all error conditions. I would remove the shiftLeft and shiftRight methods and just provide the one shift method that allows negative values. I think this would be easier for the programmer who uses the class (and the left-right naming assumes that the string is in a language that uses left-to-right writing direction). Same with shrink and grow (preferring grow).

Nb: I just noticed that I like to use "validate" instead of "check". I don't have a reason for this other than personal preference.