Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition: Palindromic Tree #205

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ahmed-dardery
Copy link

This is my implementation of palindromic tree; a strings data structure that extracts all unique palindromic substrings and their frequencies. Obviously I'm open to suggestions on how to improve it : D

Copy link
Member

@simonlindholm simonlindholm left a comment

Choose a reason for hiding this comment

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

Thanks, this seems good to have. (Closes issue #22.)

PalinTree(const string &s) : str(s) {
fail = len = pos = lz = freq = vi(sz(s) + 2);
nxt.resize(sz(s) + 2);
n = cur = fail[0] = fail[1] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

fail[0] and fail[1] are automatically 0, and n and cur can have = 0 at their declarations

* Status: stress-tested
*/
#pragma once

Copy link
Member

Choose a reason for hiding this comment

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

Indent with tabs (which will expand to 2 spaces in the pdf)

nxt.resize(sz(s) + 2);
n = cur = fail[0] = fail[1] = 0;
addNode(-1, -1), addNode(0, 0);
rep(i, 0, sz(s)) addChar(i, s[i]);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rep(i, 0, sz(s)) addChar(i, s[i]);
rep(i,0,sz(s)) addChar(i, s[i]);

(kactl doesn't use spaces in rep macro arguments except when they are long)

for (int i = n - 1; ~i; --i) {
freq[i] += lz[i];
lz[fail[i]] += lz[i];
lz[i] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

this is set up for propagate being called multiple times, but I don't see the point of that since it's O(n) which matches the full tree construction?

would it make sense to replace freq by lz, and remove this resetting?

Comment on lines +32 to +33
int u = getFailure(cur, i);
int &ch = nxt[u][c];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int u = getFailure(cur, i);
int &ch = nxt[u][c];
int u = getFailure(cur, i), &ch = nxt[u][c];

}

int getFailure(int u, int i) {
while (i <= len[u] || str[i] != str[i - len[u] - 1]) u = fail[u];
Copy link
Member

Choose a reason for hiding this comment

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

this line goes past 63 chars, wrap it before u = or golf variable names

rep(i, 0, sz(s)) addChar(i, s[i]);
propagate();
}

Copy link
Member

Choose a reason for hiding this comment

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

let's squeeze this a bit by removing blank lines between methods

int &ch = nxt[u][c];
if (~ch) return (void) ++lz[cur = ch];
int v = cur = ch = addNode(len[u] + 2, i - len[u] - 1);
fail[v] = len[v] == 1 ? 1 : nxt[getFailure(fail[u], i)][c];
Copy link
Member

Choose a reason for hiding this comment

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

this can use ch or cur instead of v (assuming getFailure is shorted to avoid line-wrapping)

@@ -8,3 +8,4 @@ \chapter{Strings}
\kactlimport{SuffixTree.h}
\kactlimport{Hashing.h}
\kactlimport{AhoCorasick.h}
\kactlimport{PalindromicTree.h}
Copy link
Member

Choose a reason for hiding this comment

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

(it's a bit sad to have both PalindromicTree and Manacher, but I guess they kinda compute different things)


int addNode(int l, int p) {
nxt[n].assign(A, -1);
len[n] = l, pos[n] = p, lz[n] = 1, freq[n] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
len[n] = l, pos[n] = p, lz[n] = 1, freq[n] = 0;
len[n] = l, pos[n] = p, lz[n] = 1;

@simonlindholm
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants