-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add a new API to get the frequency of phrases #216
base: master
Are you sure you want to change the base?
Conversation
Please help to check the CI fails. |
I will try it, but I need a little time. |
|
||
ret = sqlite3_step(pgdata->static_data.stmt_userphrase[STMT_USERPHRASE_SELECT_BY_PHONE_PHRASE]); | ||
|
||
if (ret == SQLITE_ROW) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can rewrite as following:
if (ret != SQLITE_ROW) return -1;
...
*orig_freq = sqlite3_column_int ...
Not sure what kind of test should I add, any suggestion? Thx. |
@mangokingTW : You can propose the APIs that expose statistics information for reviewing. |
You can add test case for the following scenario:
The second frequency shall not be the same as the first one. |
@mangokingTW : Please use |
I had a trip to other country for few days at last week. |
@mangokingTW : You don't have to make an apology for personal plans. Instead, you only have to reschedule and let us know. |
I will try to complete it before this weekend. |
@mangokingTW : Can you squash the commits? I think we only accept two commits:
|
It took a time for me to understand what should I do. |
@@ -529,6 +529,10 @@ CHEWING_API int chewing_userphrase_get(ChewingContext *ctx, | |||
char *phrase_buf, unsigned int phrase_len, | |||
char *bopomofo_buf, unsigned int bopomofo_len); | |||
|
|||
CHEWING_API int chewing_userphrase_get_freq(ChewingContext *ctx, | |||
const char phrase_buf[], const char bopomofo_buf[], | |||
unsigned int *orig_freq, unsigned int *max_freq, unsigned int *user_freq, unsigned int *time); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split unsigned int *user_freq, unsigned int *time
to new line for pretty code.
@kanru, do you think the proposed change still makes sense? |
I think it makes sense to expose an API to retrieve the frequency information. Currently this is already supported by the Dictionary trait in the rust version. If we are going to expose a C API, I feel it should work with the iterator interface instead of another search by bopomofo one. |
I think adding a new API to get the frequency of phrases is good for statistics or visualization.
For example, in my other project, I put a bar chart of frequency in chewing-editor.