From bbd6a26be025bc419e342e32d86629c4ebd68dd8 Mon Sep 17 00:00:00 2001 From: Keisuke Kuroyanagi Date: Fri, 26 Sep 2014 21:46:57 +0900 Subject: [PATCH] Refactoring PrevWordsInfo. Bug: 14425059 Change-Id: I48a193b965e3055bd10a00046322c2b7b19a6232 --- .../latin/DictionaryFacilitator.java | 3 +- .../inputmethod/latin/PrevWordsInfo.java | 78 ++++++++++++++----- .../UserHistoryDictionary.java | 5 +- .../latin/utils/LanguageModelParam.java | 2 +- .../inputmethod/latin/PrevWordsInfoTests.java | 66 ++++++++++++++++ .../RichInputConnectionAndTextRangeTests.java | 32 ++++---- 6 files changed, 142 insertions(+), 44 deletions(-) create mode 100644 tests/src/com/android/inputmethod/latin/PrevWordsInfoTests.java diff --git a/java/src/com/android/inputmethod/latin/DictionaryFacilitator.java b/java/src/com/android/inputmethod/latin/DictionaryFacilitator.java index aa15bd6bf..1b1162b51 100644 --- a/java/src/com/android/inputmethod/latin/DictionaryFacilitator.java +++ b/java/src/com/android/inputmethod/latin/DictionaryFacilitator.java @@ -597,8 +597,7 @@ public class DictionaryFacilitator { final SettingsValuesForSuggestion settingsValuesForSuggestion, final int sessionId) { final DictionaryGroup[] dictionaryGroups = mDictionaryGroups; final SuggestionResults suggestionResults = new SuggestionResults( - SuggestedWords.MAX_SUGGESTIONS, - prevWordsInfo.mPrevWordsInfo[0].mIsBeginningOfSentence); + SuggestedWords.MAX_SUGGESTIONS, prevWordsInfo.isBeginningOfSentenceContext()); final float[] weightOfLangModelVsSpatialModel = new float[] { Dictionary.NOT_A_WEIGHT_OF_LANG_MODEL_VS_SPATIAL_MODEL }; for (final DictionaryGroup dictionaryGroup : dictionaryGroups) { diff --git a/java/src/com/android/inputmethod/latin/PrevWordsInfo.java b/java/src/com/android/inputmethod/latin/PrevWordsInfo.java index 1b7e8f96d..1ef1bbb10 100644 --- a/java/src/com/android/inputmethod/latin/PrevWordsInfo.java +++ b/java/src/com/android/inputmethod/latin/PrevWordsInfo.java @@ -18,6 +18,7 @@ package com.android.inputmethod.latin; import android.text.TextUtils; +import com.android.inputmethod.annotations.UsedForTesting; import com.android.inputmethod.latin.utils.StringUtils; import java.util.Arrays; @@ -86,35 +87,66 @@ public class PrevWordsInfo { // For simplicity of implementation, elements may also be EMPTY_WORD_INFO transiently after the // WordComposer was reset and before starting a new composing word, but we should never be // calling getSuggetions* in this situation. - public final WordInfo[] mPrevWordsInfo; + private final WordInfo[] mPrevWordsInfo; + private final int mPrevWordsCount; // Construct from the previous word information. - public PrevWordsInfo(final WordInfo prevWordInfo) { - mPrevWordsInfo = new WordInfo[] { prevWordInfo }; + public PrevWordsInfo(final WordInfo... prevWordsInfo) { + mPrevWordsInfo = prevWordsInfo; + mPrevWordsCount = prevWordsInfo.length; } - // Construct from WordInfo array. n-th element represents (n+1)-th previous word's information. - public PrevWordsInfo(final WordInfo[] prevWordsInfo) { - mPrevWordsInfo = prevWordsInfo; + // Construct from WordInfo array and size. The caller shouldn't change prevWordsInfo after + // calling this method. + private PrevWordsInfo(final PrevWordsInfo prevWordsInfo, final int prevWordsCount) { + if (prevWordsInfo.mPrevWordsCount < prevWordsCount) { + throw new IndexOutOfBoundsException("prevWordsInfo.mPrevWordsCount (" + + prevWordsInfo.mPrevWordsCount + ") is smaller than prevWordsCount (" + + prevWordsCount + ")"); + } + mPrevWordsInfo = prevWordsInfo.mPrevWordsInfo; + mPrevWordsCount = prevWordsCount; } // Create next prevWordsInfo using current prevWordsInfo. public PrevWordsInfo getNextPrevWordsInfo(final WordInfo wordInfo) { final int nextPrevWordCount = Math.min(Constants.MAX_PREV_WORD_COUNT_FOR_N_GRAM, - mPrevWordsInfo.length + 1); + mPrevWordsCount + 1); final WordInfo[] prevWordsInfo = new WordInfo[nextPrevWordCount]; prevWordsInfo[0] = wordInfo; - System.arraycopy(mPrevWordsInfo, 0, prevWordsInfo, 1, prevWordsInfo.length - 1); + System.arraycopy(mPrevWordsInfo, 0, prevWordsInfo, 1, nextPrevWordCount - 1); return new PrevWordsInfo(prevWordsInfo); } public boolean isValid() { - return mPrevWordsInfo.length > 0 && mPrevWordsInfo[0].isValid(); + return mPrevWordsCount > 0 && mPrevWordsInfo[0].isValid(); + } + + public boolean isBeginningOfSentenceContext() { + return mPrevWordsCount > 0 && mPrevWordsInfo[0].mIsBeginningOfSentence; + } + + // n is 1-indexed. + // TODO: Remove + public CharSequence getNthPrevWord(final int n) { + if (n <= 0 || n > mPrevWordsCount) { + return null; + } + return mPrevWordsInfo[n - 1].mWord; + } + + // n is 1-indexed. + @UsedForTesting + public boolean isNthPrevWordBeginningOfSontence(final int n) { + if (n <= 0 || n > mPrevWordsCount) { + return false; + } + return mPrevWordsInfo[n - 1].mIsBeginningOfSentence; } public void outputToArray(final int[][] codePointArrays, final boolean[] isBeginningOfSentenceArray) { - for (int i = 0; i < mPrevWordsInfo.length; i++) { + for (int i = 0; i < mPrevWordsCount; i++) { final WordInfo wordInfo = mPrevWordsInfo[i]; if (wordInfo == null || !wordInfo.isValid()) { codePointArrays[i] = new int[0]; @@ -127,14 +159,12 @@ public class PrevWordsInfo { } public PrevWordsInfo getTrimmedPrevWordsInfo(final int maxPrevWordCount) { - final int newSize = Math.min(maxPrevWordCount, mPrevWordsInfo.length); - // TODO: Quit creating a new array. - final WordInfo[] prevWordsInfo = Arrays.copyOf(mPrevWordsInfo, newSize); - return new PrevWordsInfo(prevWordsInfo); + final int newSize = Math.min(maxPrevWordCount, mPrevWordsCount); + return new PrevWordsInfo(this /* prevWordsInfo */, newSize); } public int getPrevWordCount() { - return mPrevWordsInfo.length; + return mPrevWordsCount; } @Override @@ -149,16 +179,22 @@ public class PrevWordsInfo { if (!(o instanceof PrevWordsInfo)) return false; final PrevWordsInfo prevWordsInfo = (PrevWordsInfo)o; - final int minLength = Math.min(mPrevWordsInfo.length, prevWordsInfo.mPrevWordsInfo.length); + final int minLength = Math.min(mPrevWordsCount, prevWordsInfo.mPrevWordsCount); for (int i = 0; i < minLength; i++) { if (!mPrevWordsInfo[i].equals(prevWordsInfo.mPrevWordsInfo[i])) { return false; } } - final WordInfo[] longerWordsInfo = - (mPrevWordsInfo.length > prevWordsInfo.mPrevWordsInfo.length) ? - mPrevWordsInfo : prevWordsInfo.mPrevWordsInfo; - for (int i = minLength; i < longerWordsInfo.length; i++) { + final WordInfo[] longerWordsInfo; + final int longerWordsInfoCount; + if (mPrevWordsCount > prevWordsInfo.mPrevWordsCount) { + longerWordsInfo = mPrevWordsInfo; + longerWordsInfoCount = mPrevWordsCount; + } else { + longerWordsInfo = prevWordsInfo.mPrevWordsInfo; + longerWordsInfoCount = prevWordsInfo.mPrevWordsCount; + } + for (int i = minLength; i < longerWordsInfoCount; i++) { if (longerWordsInfo[i] != null && !WordInfo.EMPTY_WORD_INFO.equals(longerWordsInfo[i])) { return false; @@ -170,7 +206,7 @@ public class PrevWordsInfo { @Override public String toString() { final StringBuffer builder = new StringBuffer(); - for (int i = 0; i < mPrevWordsInfo.length; i++) { + for (int i = 0; i < mPrevWordsCount; i++) { final WordInfo wordInfo = mPrevWordsInfo[i]; builder.append("PrevWord["); builder.append(i); diff --git a/java/src/com/android/inputmethod/latin/personalization/UserHistoryDictionary.java b/java/src/com/android/inputmethod/latin/personalization/UserHistoryDictionary.java index d1486f630..121c89e83 100644 --- a/java/src/com/android/inputmethod/latin/personalization/UserHistoryDictionary.java +++ b/java/src/com/android/inputmethod/latin/personalization/UserHistoryDictionary.java @@ -71,12 +71,11 @@ public class UserHistoryDictionary extends DecayingExpandableBinaryDictionaryBas null /* shortcutTarget */, 0 /* shortcutFreq */, false /* isNotAWord */, false /* isBlacklisted */, timestamp, distracterFilter); - final boolean isBeginningOfSentenceContext = - prevWordsInfo.mPrevWordsInfo[0].mIsBeginningOfSentence; + final boolean isBeginningOfSentenceContext = prevWordsInfo.isBeginningOfSentenceContext(); final PrevWordsInfo prevWordsInfoToBeSaved = prevWordsInfo.getTrimmedPrevWordsInfo(SUPPORTED_NGRAM - 1); for (int i = 0; i < prevWordsInfoToBeSaved.getPrevWordCount(); i++) { - final CharSequence prevWord = prevWordsInfoToBeSaved.mPrevWordsInfo[i].mWord; + final CharSequence prevWord = prevWordsInfoToBeSaved.getNthPrevWord(1 /* n */); if (prevWord == null || (prevWord.length() > Constants.DICTIONARY_MAX_WORD_LENGTH)) { return; } diff --git a/java/src/com/android/inputmethod/latin/utils/LanguageModelParam.java b/java/src/com/android/inputmethod/latin/utils/LanguageModelParam.java index 7955541aa..be928077f 100644 --- a/java/src/com/android/inputmethod/latin/utils/LanguageModelParam.java +++ b/java/src/com/android/inputmethod/latin/utils/LanguageModelParam.java @@ -161,7 +161,7 @@ public final class LanguageModelParam { } final int bigramProbability = isValidWord ? BIGRAM_PROBABILITY_FOR_VALID_WORD : BIGRAM_PROBABILITY_FOR_OOV_WORD; - return new LanguageModelParam(prevWordsInfo.mPrevWordsInfo[0].mWord, word, + return new LanguageModelParam(prevWordsInfo.getNthPrevWord(1 /* n */), word, unigramProbability, bigramProbability, timestamp); } } diff --git a/tests/src/com/android/inputmethod/latin/PrevWordsInfoTests.java b/tests/src/com/android/inputmethod/latin/PrevWordsInfoTests.java new file mode 100644 index 000000000..c571d985d --- /dev/null +++ b/tests/src/com/android/inputmethod/latin/PrevWordsInfoTests.java @@ -0,0 +1,66 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.inputmethod.latin; + +import com.android.inputmethod.latin.PrevWordsInfo.WordInfo; + +import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.SmallTest; + +@SmallTest +public class PrevWordsInfoTests extends AndroidTestCase { + public void testConstruct() { + assertEquals(new PrevWordsInfo(new WordInfo("a")), new PrevWordsInfo(new WordInfo("a"))); + assertEquals(new PrevWordsInfo(WordInfo.BEGINNING_OF_SENTENCE), + new PrevWordsInfo(WordInfo.BEGINNING_OF_SENTENCE)); + assertEquals(new PrevWordsInfo(WordInfo.EMPTY_WORD_INFO), + new PrevWordsInfo(WordInfo.EMPTY_WORD_INFO)); + assertEquals(new PrevWordsInfo(WordInfo.EMPTY_WORD_INFO), + new PrevWordsInfo(WordInfo.EMPTY_WORD_INFO)); + } + + public void testIsBeginningOfSentenceContext() { + assertFalse(new PrevWordsInfo().isBeginningOfSentenceContext()); + assertTrue(new PrevWordsInfo(WordInfo.BEGINNING_OF_SENTENCE) + .isBeginningOfSentenceContext()); + assertTrue(PrevWordsInfo.BEGINNING_OF_SENTENCE.isBeginningOfSentenceContext()); + assertFalse(new PrevWordsInfo(new WordInfo("a")).isBeginningOfSentenceContext()); + assertFalse(new PrevWordsInfo(new WordInfo("")).isBeginningOfSentenceContext()); + assertFalse(new PrevWordsInfo(WordInfo.EMPTY_WORD_INFO).isBeginningOfSentenceContext()); + assertTrue(new PrevWordsInfo(WordInfo.BEGINNING_OF_SENTENCE, new WordInfo("a")) + .isBeginningOfSentenceContext()); + assertFalse(new PrevWordsInfo(new WordInfo("a"), WordInfo.BEGINNING_OF_SENTENCE) + .isBeginningOfSentenceContext()); + assertFalse(new PrevWordsInfo(WordInfo.EMPTY_WORD_INFO, WordInfo.BEGINNING_OF_SENTENCE) + .isBeginningOfSentenceContext()); + } + + public void testGetNextPrevWordsInfo() { + final PrevWordsInfo prevWordsInfo_a = new PrevWordsInfo(new WordInfo("a")); + final PrevWordsInfo prevWordsInfo_b_a = + prevWordsInfo_a.getNextPrevWordsInfo(new WordInfo("b")); + assertEquals("b", prevWordsInfo_b_a.getNthPrevWord(1)); + assertEquals("a", prevWordsInfo_b_a.getNthPrevWord(2)); + final PrevWordsInfo prevWordsInfo_bos_b = + prevWordsInfo_b_a.getNextPrevWordsInfo(WordInfo.BEGINNING_OF_SENTENCE); + assertTrue(prevWordsInfo_bos_b.isBeginningOfSentenceContext()); + assertEquals("b", prevWordsInfo_bos_b.getNthPrevWord(2)); + final PrevWordsInfo prevWordsInfo_c_bos = + prevWordsInfo_b_a.getNextPrevWordsInfo(new WordInfo("c")); + assertEquals("c", prevWordsInfo_c_bos.getNthPrevWord(1)); + } +} diff --git a/tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java b/tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java index f9d72269e..2712dc228 100644 --- a/tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java +++ b/tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java @@ -30,7 +30,6 @@ import android.view.inputmethod.ExtractedTextRequest; import android.view.inputmethod.InputConnection; import android.view.inputmethod.InputConnectionWrapper; -import com.android.inputmethod.latin.PrevWordsInfo.WordInfo; import com.android.inputmethod.latin.settings.SpacingAndPunctuations; import com.android.inputmethod.latin.utils.PrevWordsInfoUtils; import com.android.inputmethod.latin.utils.RunInLocale; @@ -159,25 +158,24 @@ public class RichInputConnectionAndTextRangeTests extends AndroidTestCase { public void testGetPreviousWord() { // If one of the following cases breaks, the bigram suggestions won't work. assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def", mSpacingAndPunctuations, 2).mPrevWordsInfo[0].mWord, "abc"); + "abc def", mSpacingAndPunctuations, 2).getNthPrevWord(1), "abc"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( "abc", mSpacingAndPunctuations, 2), PrevWordsInfo.BEGINNING_OF_SENTENCE); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( "abc. def", mSpacingAndPunctuations, 2), PrevWordsInfo.BEGINNING_OF_SENTENCE); assertFalse(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def", mSpacingAndPunctuations, 2).mPrevWordsInfo[0].mIsBeginningOfSentence); + "abc def", mSpacingAndPunctuations, 2).isBeginningOfSentenceContext()); assertTrue(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc", mSpacingAndPunctuations, 2).mPrevWordsInfo[0].mIsBeginningOfSentence); + "abc", mSpacingAndPunctuations, 2).isBeginningOfSentenceContext()); // For n-gram assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def", mSpacingAndPunctuations, 1).mPrevWordsInfo[0].mWord, "def"); + "abc def", mSpacingAndPunctuations, 1).getNthPrevWord(1), "def"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def", mSpacingAndPunctuations, 1).mPrevWordsInfo[1].mWord, "abc"); - assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def", mSpacingAndPunctuations, 2).mPrevWordsInfo[1], - WordInfo.BEGINNING_OF_SENTENCE); + "abc def", mSpacingAndPunctuations, 1).getNthPrevWord(2), "abc"); + assertTrue(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( + "abc def", mSpacingAndPunctuations, 2).isNthPrevWordBeginningOfSontence(2)); // The following tests reflect the current behavior of the function // RichInputConnection#getNthPreviousWord. @@ -187,20 +185,20 @@ public class RichInputConnectionAndTextRangeTests extends AndroidTestCase { // logical. These tests are just there to catch any unintentional // changes in the behavior of the RichInputConnection#getPreviousWord method. assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def ", mSpacingAndPunctuations, 2).mPrevWordsInfo[0].mWord, "abc"); + "abc def ", mSpacingAndPunctuations, 2).getNthPrevWord(1), "abc"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def.", mSpacingAndPunctuations, 2).mPrevWordsInfo[0].mWord, "abc"); + "abc def.", mSpacingAndPunctuations, 2).getNthPrevWord(1), "abc"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def .", mSpacingAndPunctuations, 2).mPrevWordsInfo[0].mWord, "def"); - assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc ", mSpacingAndPunctuations, 2), PrevWordsInfo.BEGINNING_OF_SENTENCE); + "abc def .", mSpacingAndPunctuations, 2).getNthPrevWord(1), "def"); + assertTrue(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( + "abc ", mSpacingAndPunctuations, 2).isBeginningOfSentenceContext()); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def", mSpacingAndPunctuations, 1).mPrevWordsInfo[0].mWord, "def"); + "abc def", mSpacingAndPunctuations, 1).getNthPrevWord(1), "def"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc def ", mSpacingAndPunctuations, 1).mPrevWordsInfo[0].mWord, "def"); + "abc def ", mSpacingAndPunctuations, 1).getNthPrevWord(1), "def"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( - "abc 'def", mSpacingAndPunctuations, 1).mPrevWordsInfo[0].mWord, "'def"); + "abc 'def", mSpacingAndPunctuations, 1).getNthPrevWord(1), "'def"); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord( "abc def.", mSpacingAndPunctuations, 1), PrevWordsInfo.BEGINNING_OF_SENTENCE); assertEquals(PrevWordsInfoUtils.getPrevWordsInfoFromNthPreviousWord(