22050111045 - Cüneyt Şahin. 22050111069-Berat Mut#56
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements string matching algorithm homework by students Cüneyt Şahin (22050111045) and Berat Mut (22050111069). The submission includes implementations of Boyer-Moore and Sunday algorithms, along with a pre-analysis strategy for algorithm selection.
Changes:
- Implemented algorithm selection logic based on pattern length, repetition, and alphabet size
- Completed Boyer-Moore string matching algorithm implementation
- Completed GoCrazy (Sunday Algorithm) string matching implementation
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/PreAnalysis.java | Implements chooseAlgorithm with heuristics for selecting between Naive, KMP, and GoCrazy algorithms based on pattern characteristics |
| src/Analysis.java | Implements Boyer-Moore bad character heuristic and Sunday algorithm with Unicode support |
| my_journey.pdf | Added PDF document (likely project documentation or report) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (m == 0) { | ||
| for (int i = 0; i <= n; i++) indices.add(i); | ||
| return indicesToString(indices); |
There was a problem hiding this comment.
When the pattern is empty (m == 0), the code adds all positions from 0 to n (inclusive) to the indices list. This means for a text of length n, it adds n+1 indices. While this might be the intended behavior for empty pattern matching, it's unusual and potentially inefficient. Verify that this is the correct specification for empty pattern handling, as typically empty patterns either match nowhere or only at the beginning.
| //Here we determine table size ASCII or Unicode | ||
| int tableSize = 256; | ||
| for (int i = 0; i < m; i++) { | ||
| if (pattern.charAt(i) >= 256) { | ||
| tableSize = 65536; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| int[] badChar = new int[tableSize]; | ||
| Arrays.fill(badChar, -1); | ||
|
|
||
|
|
||
| for (int i = 0; i < m; i++) { | ||
|
|
||
| badChar[pattern.charAt(i)] = i; | ||
| } |
There was a problem hiding this comment.
The tableSize is set to 65536 for Unicode characters, which could cause significant memory usage (256 KB for a single integer array). For patterns with even one Unicode character, this approach allocates a large array that may be mostly empty. Consider using a HashMap for the bad character table when Unicode characters are detected, similar to how the GoCrazy algorithm handles it, to save memory.
| j++; | ||
| } | ||
|
|
||
| //If full match found record index |
There was a problem hiding this comment.
The comment on line 320 states "If full match found record index" but is missing proper punctuation. It should end with a comma or period, such as "If full match found, record index" or "Full match found - record index".
| //If full match found record index | |
| // If full match found, record index |
| return null; // Return null to run all algorithms, or return algorithm name to use pre-analysis | ||
| int n = text.length(); | ||
| int m = pattern.length(); | ||
| if (m <=4) { |
There was a problem hiding this comment.
The condition checks if pattern length is less than or equal to 4, but the isRepetitive method has a guard that returns false for patterns shorter than 5 characters. This means patterns of exactly length 4 will be handled by Naive (which is fine), but there's an inconsistency in the logic flow. Consider whether the threshold of 4 is appropriate, or if it should be adjusted to match the isRepetitive threshold.
| if (m <=4) { | |
| if (m < 5) { |
| if (m < 5) return false; | ||
|
|
||
|
|
||
| int limit = Math.min(m, 8); | ||
| char first = pattern.charAt(0); | ||
| int count = 0; | ||
|
|
||
| for (int i = 0; i < limit; i++) { | ||
| if (pattern.charAt(i) == first) { | ||
| count++; | ||
| } | ||
| } | ||
|
|
||
| return count >= (limit * 0.75); |
There was a problem hiding this comment.
The isRepetitive method only checks if the first character repeats frequently in the first 8 characters. This is a very limited heuristic for detecting repetitive patterns. A truly repetitive pattern might have structures like "ababab" or "abcabc" which wouldn't be caught by this logic. Consider renaming the method to be more specific about what it checks, such as isFirstCharacterRepetitive, or implement a more comprehensive repetition detection algorithm.
| if (m < 5) return false; | |
| int limit = Math.min(m, 8); | |
| char first = pattern.charAt(0); | |
| int count = 0; | |
| for (int i = 0; i < limit; i++) { | |
| if (pattern.charAt(i) == first) { | |
| count++; | |
| } | |
| } | |
| return count >= (limit * 0.75); | |
| // Very short patterns cannot meaningfully be considered repetitive | |
| if (m < 2) { | |
| return false; | |
| } | |
| // Check if the pattern is composed of repeated occurrences of a smaller prefix. | |
| // For each possible period length p, verify that the string is periodic with period p. | |
| for (int p = 1; p <= m / 2; p++) { | |
| if (m % p != 0) { | |
| continue; | |
| } | |
| boolean periodic = true; | |
| for (int i = p; i < m; i++) { | |
| if (pattern.charAt(i) != pattern.charAt(i % p)) { | |
| periodic = false; | |
| break; | |
| } | |
| } | |
| if (periodic) { | |
| return true; | |
| } | |
| } | |
| return false; |
| int n = text.length(); | ||
| int m = pattern.length(); | ||
|
|
||
| //We handle edge cases here: empty pattern or pattern > text |
There was a problem hiding this comment.
The comment on line 210 has a grammatical error. It should read "We handle edge cases here" instead of "We handle edge cases here:" (the colon suggests a list follows, but the actual cases are on separate lines). Consider removing the colon or restructuring the comment to properly introduce the edge case handling.
| //We handle edge cases here: empty pattern or pattern > text | |
| //We handle edge cases here for empty pattern or pattern > text |
|
|
||
| //Here we handle edge cases | ||
| if (m == 0) { | ||
| for (int i = 0; i <= n; i++) indices.add(i); |
There was a problem hiding this comment.
In the GoCrazy (Sunday Algorithm) implementation, when handling the empty pattern case, the same issue exists as in BoyerMoore: it adds n+1 indices (from 0 to n inclusive). This should be consistent with the BoyerMoore implementation and verified to match the expected behavior for empty patterns.
| for (int i = 0; i <= n; i++) indices.add(i); | |
| for (int i = 0; i < n; i++) indices.add(i); |
No description provided.