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

Optionally analyze compound word parts #8

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Conversation

mortterna
Copy link
Member

No description provided.

@mortterna
Copy link
Member Author

Täytyy nyt ainakin releasen versionumerot katsoa. Readme:n voisi ehkä myös näistä jotain versioista satuilla.

@mortterna mortterna force-pushed the feature/compound-word-parts branch 2 times, most recently from ed16e2b to bb705a5 Compare January 27, 2023 14:04
List<String> baseFormParts = analysis.getBaseFormParts();

if (baseForm != null && !results.contains(baseForm)) {
results.add(baseForm);
Copy link
Member

Choose a reason for hiding this comment

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

Alla tsekataan duplikaatit, mutta tässä ei. Pitäisikö?

Copy link
Member Author

Choose a reason for hiding this comment

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

En ymmärrä kommenttia. Duplikaattia on yritetty ainakin tarkastaa, mutta onkohan implementaatiossa siis joku bugi?

Copy link
Member

Choose a reason for hiding this comment

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

En mä vaan tiedä, varmaan silmät harittaa jotenkin. 😄

}
}
}
}

switch (results.size()) {
Copy link
Member

Choose a reason for hiding this comment

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

Nykyisellään tää switch ei enää aiheuta kuin overheadia; results:n voisi palauttaa suoraan. Aiemmin se oli vain allokoinnin vähentämistä tilanteessa jossa tuloksia ei tullut lainkaan.

Mutta ehkä tässäkin voisi vielä ennen luuppia sanoa:

List<Analysis> analysisResults = raudikkoAnalyzer.analyze(word);
if (analysisResults.isEmpty())
    return emptyList();

Toiminnallisesti ihan sama kuin ilmankin, mutta välttää sekä iteraattorin että tuloslistan allokoinnin. (Iteraattori nyt varmaan menee escape analyysillä stackiin, mutta silti.)

Ei nyt normaalissa koodissa viitsisi tällä tasolla viilata, mutta kun tätä kutsutaan kymmeniä miljoonia kertoja, niin kaikki on plussaa.

List<String> baseFormParts = analysis.getBaseFormParts();

if (baseForm != null && !results.contains(baseForm)) {
results.add(baseForm);
Copy link
Member

Choose a reason for hiding this comment

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

En mä vaan tiedä, varmaan silmät harittaa jotenkin. 😄

@komu komu merged commit 6a1365a into master Jan 31, 2023
@komu komu deleted the feature/compound-word-parts branch January 31, 2023 13:40
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.

2 participants