###Don't add unnecessary pre-checks
Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
###Use built-in methods
Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);
###Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
###Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);
Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);
###Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
###Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);
###Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
###Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);
###Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
###Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);
###Don't add unnecessary pre-checks
Your method getSubstringsSet
starts with two early returns:
if(aString.length() < lengthOfSubstring){
return setOfSubstrings;
}
if(aString.length() == lengthOfSubstring){
setOfSubstrings.add(aString);
return setOfSubstrings;
}
You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
###Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();
//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
sb.append(charArray[i + j]);
}
setOfSubstrings.add(sb.toString());
So you're creating a new StringBuilder
and then appending each character. This is a complicated way of retrieving the substring
between two indices. It also allocates a new StringBuilder
each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));
In the same way, stringIntersect
is a lot more complicated than it should be.
First of all, you are using a label outerForLoop
and then using it to break out of the inner loop with break outerForLoop;
. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2)
that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint
. This method returns true
if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);