Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion. See this related discussion.)
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Remove unnecessary elements
You're not using the private constructor. So drop it.
This validation seems pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
Without this, your method will still work fine, and return an empty set for an empty string input, which makes sense. So I think you can drop this.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion.)
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Remove unnecessary elements
You're not using the private constructor. So drop it.
This validation seems pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
Without this, your method will still work fine, and return an empty set for an empty string input, which makes sense. So I think you can drop this.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion.)
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Remove unnecessary elements
You're not using the private constructor. So drop it.
This validation seems pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
Without this, your method will still work fine, and return an empty set for an empty string input, which makes sense. So I think you can drop this.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion.)
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Unused thingsRemove unnecessary elements
You're not using the private constructor. So drop it. Maybe you created it because it's a "utility class", but it should not be a utility class. Following the advice of @Simon André Forsberg this could be a regular class, you could have a common interface for this functionality with different possible implementations.
This validation seems quite pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
I think you can just drop it. YourWithout this, your method will still work just fine with an empty string, and return an empty set for an empty string input, which makes sense. So I think you can drop this.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion.)
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Unused things
You're not using the private constructor. So drop it. Maybe you created it because it's a "utility class", but it should not be a utility class. Following the advice of @Simon André Forsberg this could be a regular class, you could have a common interface for this functionality with different possible implementations.
This validation seems quite pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
I think you can just drop it. Your method will work just fine with an empty string and return an empty set, which makes sense.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion.)
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Remove unnecessary elements
You're not using the private constructor. So drop it.
This validation seems pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
Without this, your method will still work fine, and return an empty set for an empty string input, which makes sense. So I think you can drop this.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion. )
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Unused things
You're not using the private constructor. So drop it. Maybe you created it because it's a "utility class", but it should not be a utility class. Following the advice of @Simon André Forsberg this could be a regular class, you could have a common interface for this functionality with different possible implementations.
This validation seems quite pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
I think you can just drop it. Your method will work just fine with an empty string and return an empty set, which makes sense.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time.
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Unused things
You're not using the private constructor. So drop it. Maybe you created it because it's a "utility class", but it should not be a utility class. Following the advice of @Simon André Forsberg this could be a regular class, you could have a common interface for this functionality with different possible implementations.
This validation seems quite pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
I think you can just drop it. Your method will work just fine with an empty string and return an empty set, which makes sense.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.
Efficiency
You're using a TreeSet to hold the valid words, but you're not using its key feature: ordering. As such, it would be more efficient to use a HashSet instead. That will give constant time lookups, as opposed to logarithmic time. (See this related discussion. )
The algorithm is definitely wasteful. If for example the longest word in the dictionary is 3 characters long and the input text is 50, the for loop will happily try all substrings, even if most of them will be longer than 3. As a small speed improvement you could pre-calculate the minimum and maximum word lengths when you create the dictionary, and include extra conditions in the for loop using those values.
Another improvement could be, if memory permits, to build a dictionary of all possible prefixes, with signature Map<Integer, Set<String>> where the key is the length of a prefix, mapped to the set of prefixes of that length. Then in the for loop, you could check if the current substring is contained in the prefix dictionary, otherwise break the innermost loop and continue from the next i + 1.
Probably there is a more elegant logic to solve this problem without a brute force approach but I can't think of it right now...
Unused things
You're not using the private constructor. So drop it. Maybe you created it because it's a "utility class", but it should not be a utility class. Following the advice of @Simon André Forsberg this could be a regular class, you could have a common interface for this functionality with different possible implementations.
This validation seems quite pointless:
if (str.length() == 0) {
throw new IllegalArgumentException("Strings of length 0 are illegal");
}
I think you can just drop it. Your method will work just fine with an empty string and return an empty set, which makes sense.
Unit testing
Add more unit tests for all possible corner cases you can think of. For example empty string. Or string with words not in the dictionary. Each of these in a separate test case.