Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 82 additions & 7 deletions src/main/java/StringManipulation.java

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jon, I've added more comments in addition to the those added 6/16. Overall, your code is clean and easy to follow, with most of my comments reading more as differences of opinion than suggestions to change anything. I especially like how you leverage the java.util library to accomplish the objectives of the assignment. After the C++ we learned in past classes, I forgot how important builtin libraries are. While some may critique this as unnecessary code coupling, I truly see it as an excellent example of code reuse, saving dev time. The one major recommendation I have is to add more variety to the test cases. Otherwise, great work!

Original file line number Diff line number Diff line change
@@ -1,32 +1,107 @@
/*
Jon Kaimmer
CS410 Software Engineering
Spring 2023
Junit Testing Assignment

Spring Manipulation
*/

import java.lang.IllegalArgumentException;
import java.lang.IndexOutOfBoundsException;
import java.lang.NullPointerException;
import java.util.*;


public class StringManipulation implements StringManipulationInterface {

public String s;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing String from public to protected/private here would increase code encapsulation.


@Override
public String getString() {
return null;
return s;
}

@Override
public void setString(String string) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the event that user sets string = "", consider overriding that input with null? May be outside of how Sara expects us to implement, though perhaps worth considering for methods below. Save on calling isEmpty(). For sure up for interpretation, no single answer.

s = string;
}

@Override
public int count() {
return 0;
if (s == null || s.isEmpty()){
return 0;
}
String[] words = s.split("\\s+");
return words.length;
}

@Override
public String removeNthCharacter(int n, boolean maintainSpacing) {
return null;

if(s == null){
throw new NullPointerException("string cannot be null");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null ptr exception not mentioned in the interface for the assigment.
Throw indexoutofbound? null str has no words

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you are probably right, I just wanted to add exception checking and figured that a string might be null so why not check whether the string was nulll.. indexOutOfBounds is a good suggestion

}
if (n <= 0) {
throw new IllegalArgumentException("n must be greater than zero.");
}
if (n > s.length()){
throw new IndexOutOfBoundsException("n is greater than the string length.");
}

StringBuilder sb = new StringBuilder();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can instantiate StringBuilder() by passing an existing string to it. After, you can call StringBuilder.remove(int index) to remove characters at a given position. If maintainSpacing == true, StringBuilder.setCharAt(int index, char ch) can be called to replace chars with ' '.


for (int i=0 ; i < s.length() ; i++){
if((i+1)%n == 0){
if (maintainSpacing){
sb.append(' ');
}
} else {
sb.append(s.charAt(i));
}
}

return sb.toString();
}

@Override
public String[] getSubStrings(int startWord, int endWord) {
return null;
public String[] getSubStrings(int startWord, int endWord){

if (startWord <= 0 || endWord <= 0 || startWord > endWord) {
throw new IllegalArgumentException("Invalid input: startWord and endWord should be greater than 0, and startWord should not be greater than endWord.");
}

String[] words = s.split("\\s+"); // Split the string into words

if (endWord > words.length) {
throw new IndexOutOfBoundsException("The string has less than " + endWord + " words in it.");
}

String[] subStrings = new String[endWord - startWord + 1];

for (int i = startWord - 1; i < endWord; i++) { // Array indices start at 0, but word positions start at 1
subStrings[i - startWord + 1] = words[i]; // Adjust indices for the same reason
}

return subStrings;
}

@Override
public String restoreString(int[] indices) {
return null;
public String restoreString(int[] indices){
if (s.length() != indices.length) {
throw new IllegalArgumentException("The length of the string and the indices array must be the same.");
}

char[] shuffled = new char[s.length()];

for (int i = 0; i < s.length(); i++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move exception checking up? save on shuffling in event that exception is thrown later? idk may not be computationally significant i'm just writing words

if (indices[i] < 0 || indices[i] >= s.length()) {
throw new IndexOutOfBoundsException("Index " + indices[i] + " is out of bounds.");
}
shuffled[indices[i]] = s.charAt(i);
}

return new String(shuffled);
}


Expand Down
158 changes: 112 additions & 46 deletions src/test/java/StringManipulationTest.java
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
/*
Jon Kaimmer
CS410 Software Engineering
Spring 2023
Junit Testing Assignment

Spring Manipulation Test
*/

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -20,66 +29,87 @@ public void tearDown() {
}

@Test

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if string contains non alphanumeric chars? no test case but maybe that's not even in the scope of this assignment who knows?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That edge case was out of the bounds of the assignment.
Future improvement for other classes for sure

public void testCount1() {
public void testCount1_NormalString() {

@mihaisiia mihaisiia Jun 19, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count() test cases provided for alphabet/empty/null strings, further test cases by Sara suggest string provided may include numbers interspersed with letters, consider adding a test case for this scenario.

Edit: I realized that this comment is just rehashing what I wrote in the comment above this one, but I will leave it up anyways.

manipulatedstring.setString("This is my string");
int length = manipulatedstring.count();
assertEquals(4, length);
}

@Test
public void testCount2() {
fail("Not yet implemented");
public void testCount2_EmptyString() {
manipulatedstring.setString("");
int length = manipulatedstring.count();
assertEquals(0, length);
}

@Test
public void testCount3() {
fail("Not yet implemented");
public void testCount3_Null() {
manipulatedstring.setString(null);
int length = manipulatedstring.count();
assertEquals(0, length);
}

@Test
public void testCount4() {
fail("Not yet implemented");
public void testCount4_TabSeperatedWords() {
manipulatedstring.setString("This is my string");
int length = manipulatedstring.count();
assertEquals(4, length);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should removeNthCharacter(1, true/false) behave? Should the method return "" or null?

@Test
public void testRemoveNthCharacter1() {
public void testRemoveNthCharacter1_DontMaintainSpacing() {
manipulatedstring.setString("I'd b3tt3r put s0me d161ts in this 5tr1n6, right?");
assertEquals("I' bttr uts0e 16tsinths trn6 rgh?", manipulatedstring.removeNthCharacter(3, false));
}

@Test
public void testRemoveNthCharacter2() {
public void testRemoveNthCharacter2_DoMaintainSpacing() {
manipulatedstring.setString("I'd b3tt3r put s0me d161ts in this 5tr1n6, right?");
assertEquals("I' b tt r ut s0 e 16 ts in th s tr n6 r gh ?", manipulatedstring.removeNthCharacter(3, true));
}

@Test
public void testRemoveNthCharacter3() {
fail("Not yet implemented");
}
public void testRemoveNthCharacter3_EmptyString_IndexOutOfBoundsException() {
manipulatedstring.setString("");

@Test
public void testRemoveNthCharacter4() {
fail("Not yet implemented");
assertThrows(IndexOutOfBoundsException.class,
()-> {
manipulatedstring.removeNthCharacter(1, false);
});
}

@Test
public void testRemoveNthCharacter5() {
fail("Not yet implemented");
public void testRemoveNthCharacter4_NullString_ThrowNullPointerException() {
manipulatedstring.setString(null);

assertThrows(NullPointerException.class,
()-> {
manipulatedstring.removeNthCharacter(1, false);
});
}

@Test
public void testRemoveNthCharacter6() {
fail("Not yet implemented");
public void testRemoveNthCharacter5_nEqualsZero_ThrowIllegalArgument() {
manipulatedstring.setString("I am a String!");

assertThrows(IllegalArgumentException.class,
()-> {
manipulatedstring.removeNthCharacter(0, false);
});
}

@Test
public void testRemoveNthCharacter7() {
fail("Not yet implemented");
public void testRemoveNthCharacter6_nLargerThanString_ThrowIndexOutOfBounds() {
manipulatedstring.setString("I am a String!");

assertThrows(IndexOutOfBoundsException.class,
()-> {
manipulatedstring.removeNthCharacter(15, false);
});
}

@Test
public void testGeSubStrings1() {
public void testGetSubStrings1_TestWords() {
manipulatedstring.setString("This is my string");
String [] sStings = manipulatedstring.getSubStrings(3, 4);

Expand All @@ -88,28 +118,53 @@ public void testGeSubStrings1() {
}

@Test
public void testGeSubStrings2() {
fail("Not yet implemented");
public void testGetSubStrings2_ThrowIllegalArgumentException_startWordLessThanZero() {
manipulatedstring.setString("I am a String!");

assertThrows(IllegalArgumentException.class,
()-> {
String [] sStings = manipulatedstring.getSubStrings(-1, 4);
});
}
@Test
public void testGeSubStrings3() {
fail("Not yet implemented");
public void testGetSubStrings3_ThrowIllegalArgumentException_endWordLessThanZero() {
manipulatedstring.setString("I am a String!");

assertThrows(IllegalArgumentException.class,
()-> {
String [] sStings = manipulatedstring.getSubStrings(1, -1);
});
}
@Test
public void testGeSubStrings4() {
fail("Not yet implemented");
public void testGetSubStrings4_ThrowIllegalArgumentException_endWordLessThanstartWord() {
manipulatedstring.setString("I am a String!");

assertThrows(IllegalArgumentException.class,
()-> {
String [] sStings = manipulatedstring.getSubStrings(2, 1);
});
}
@Test
public void testGeSubStrings5() {
fail("Not yet implemented");
public void testGetSubStrings5_ThrowIndexOutOfBoundsException_endWordGreaterThanNumberOfWords() {
manipulatedstring.setString("I am a String!");

assertThrows(IndexOutOfBoundsException.class,
()-> {
String [] sStings = manipulatedstring.getSubStrings(1, 5);
});
}
@Test
public void testGeSubStrings6() {
fail("Not yet implemented");
public void testGetSubStrings6_EmptyString() {
manipulatedstring.setString("");

assertThrows(IndexOutOfBoundsException.class,
()-> {
String [] sStings = manipulatedstring.getSubStrings(1, 5);
});
}

@mihaisiia mihaisiia Jun 19, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All RestoreString() test cases use the same string, consider adding tests that deal with numbers/symbols/whitespace.

@Test
public void testRestoreString1()
public void testRestoreString1_3LetterWord()
{
manipulatedstring.setString("art");
int [] array;
Expand All @@ -119,31 +174,42 @@ public void testRestoreString1()
}

@Test
public void testRestoreString2()
{
fail("Not yet implemented");

}

@Test
public void testRestoreString3()
public void testRestoreString2_ThrowIllegalArgumentException()
{
fail("Not yet implemented");
manipulatedstring.setString("art");
int [] array;
array=new int[]{1,0};

assertThrows(IllegalArgumentException.class,
()-> {
String restoreString = manipulatedstring.restoreString(array);
});
}

@Test
public void testRestoreString4()
public void testRestoreString3_ThrowIndexOutOfBoundsException_GreaterThan()
{
fail("Not yet implemented");
manipulatedstring.setString("art");
int [] array;
array=new int[]{1,0,10};

assertThrows(IndexOutOfBoundsException.class,
()-> {
String restoreString = manipulatedstring.restoreString(array);
});
}

@Test
public void testRestoreString5()
public void testRestoreString4_ThrowIndexOutOfBoundsException_LessThan()
{
fail("Not yet implemented");
manipulatedstring.setString("art");
int [] array;
array=new int[]{1,0,-10};

assertThrows(IndexOutOfBoundsException.class,
()-> {
String restoreString = manipulatedstring.restoreString(array);
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the exception of testCount(), all of the test cases check if exceptions are thrown. It may be worth adding unique test cases that test your program logic to ensure that your methods operate as you intend them to.


}