Synonymiser рефакторинг


Мне интересно, как люди будут рефакторить этот код, чтобы сделать его более читабельным, удалить локальные переменные и т. д. Код проверяет строку на ряд массивов. Первый (0) поле каждого массива-это корень слова, а остальные слова являются синонимами. Идея в том, чтобы заменить любое слово в пользовательскую строку ключевое слово или предоставить пользовательские слова, если ни один не был найден.

import java.util.Arrays;

public class Synonymiser {

    private final static int ROOT_WORD = 0;

    private String[][] mSynonyms;

    public Synonymiser(String[]... synonyms) {
        super();
        this.mSynonyms = synonyms;
    }


    public String normaliseString(String inStr){
        String[] inArr = inStr.split(" ");
        boolean matchFound;

        for (int i = 0; i < inArr.length; i++){
            matchFound=false;
            for (String[] synonymArr : mSynonyms){
                 for(int j = 0 ; j < synonymArr.length; j++){
                     if (inArr[i].equalsIgnoreCase(synonymArr[j])){
                         inArr[i] = synonymArr[ROOT_WORD];
                         matchFound = true;
                     }
                     if (matchFound) break; 
                 }
                 if (matchFound)  break; 
            }
        }
        return Arrays.toString(inArr);
    }

    public static void main(String [] args){
        String[] ar1 = new String[] {"one", "two", "three", "four", "five"};
        String[] ar2 = new String[] {"six", "seven", "eight"};
        Synonymiser s = new Synonymiser(ar1, ar2);

        System.out.println(s.normaliseString("one two seven bulb"));
    }

}


750
4
задан 21 декабря 2011 в 07:12 Источник Поделиться
Комментарии
5 ответов

1, Вы сможете устранить matchFound флаг, если вы создать ярлык для вашего внешнего на петли и вызвать разрыв с меткой:

matchLoop: for (final String[] synonymArr : mSynonyms) {
for (int j = 0; j < synonymArr.length; j++) {
if (inArr[i].equalsIgnoreCase(synonymArr[j])) {
inArr[i] = synonymArr[ROOT_WORD];
break matchLoop;
}
}
}

В любом случае, я думаю, что с помощью метки, является более или менее неприятный запах, поэтому постарайтесь извлечь из петель на отдельные методы.

2, я бы создать хотя бы слово класс, который хранит (корень)Слово и его синонимы. Я должен привести более читаемый код в Synonymiser класса:

public class Word {

private final String rootWord;

private final Set<String> synomins;

public Word(final String rootWord, final Collection<String> synomins) {
super();
// TODO: empty String/null check
this.rootWord = rootWord;
// TODO: convert the input to lowercase (it helps contains())
this.synomins = new HashSet<String>(synomins);
this.synomins.add(rootWord);
}

public static Word createWord(final String rootWord, final String... synonims) {
final List<String> synonimList = Arrays.asList(synonims);
return new Word(rootWord, synonimList);
}

public boolean contains(final String word) {
// TODO: convert word to lowercase for proper comparison
if (synomins.contains(word)) {
return true;
}
return false;
}

public String getRootWord() {
return rootWord;
}
}

1
ответ дан 21 декабря 2011 в 09:12 Источник Поделиться

Я хотел бы пойти дальше, чем просто сделать красивее код, и изменить способ, который вы используете. Вместо перебора массивов в поисках совпадений, можно поставить синонимы в хэшированном список. Это даст вам представление близко к O(1) для каждой подстановки, вместо o(n) времени.

Пример на C#:

public class Synonymiser {

private Dictionary<string, string> _synonyms = new Dictionary<string, string>();

public Synonymiser AddWord(string word, params string[] synonyms) {
foreach (string synonym in synonyms) {
_synonyms.Add(synonym.ToUpper(), word);
}
return this;
}

public string NormaliseString(string inStr){
string replacement;
return String.Join(
" ",
inStr.Split(' ')
.Select(w => _synonyms.TryGetValue(w.ToUpper(), out replacement) ? replacement : w)
);
}

}

Использование:

Synonymiser s =
new Synonymiser()
.AddWord("one", "two", "three", "four", "five")
.AddWord("six", "seven", "eight");

Console.WriteLine(s.NormaliseString("one two seven bulb"));

Выход:

one one six bulb

5
ответ дан 21 декабря 2011 в 10:12 Источник Поделиться

Эта реализация будет иметь низкую производительность для больших словарей - вам будет o(n) производительность для n корень слова. Я бы порекомендовал преобразования массива синоним в HashMap, сопоставления ключевых слов к своему корню значения слова. Это даст вам немного дороже инициализацию (по сути, цена, которую вы платите за каждый вызов normalizeString будет выплачиваться раз, на строительство). Объем памяти будет немного больше, а также один дополнительный указатель на синоним.

import java.util.Arrays;
import java.util.HashMap;

public class Synonymiser {

private final static int ROOT_WORD = 0;

private HashMap<String, String> mSynonyms;

public Synonymiser(String[]... synonyms) {
super();
mSynonyms = new HashMap<String, String>(synonyms.length*5); //Enough initial storage for 5 synonyms per root
for(String[] root: synonyms) {
for(int i = 0; i<root.length; ++i) {
if(i == ROOT_WORD) {
continue;
}
mSynonyms.put(root[i], root[ROOT_WORD]);
}
}
}

public String normaliseString(String inStr){
String[] inArr = inStr.split(" ");

for (int i = 0; i < inArr.length; i++){
String synonym = mSynonyms.get(inArr[i]);
if(synonym != null) {
inArr[i] = synonym;
}
}
return Arrays.toString(inArr);
}

public static void main(String [] args){
String[] ar1 = new String[] {"one", "two", "three", "four", "five"};
String[] ar2 = new String[] {"six", "seven", "eight"};
Synonymiser s = new Synonymiser(ar1, ar2);

System.out.println(s.normaliseString("one two seven bulb"));
}

}

2
ответ дан 21 декабря 2011 в 02:12 Источник Поделиться

Я бы для начала извлечь тело из петли крайние. Для любой заданной строки кандидата, найти синоним.

public class Synonymiser {

...

public String normaliseString(String inStr){
String[] inArr = inStr.split(" ");
for (int i = 0; i < inArr.length; i++) {
inArr[i] = synonym(inArr[i]);
}
return Arrays.toString(inArr);
}

private String synonym(String candidate) {
for (String[] array : mSynonyms) {
for (String s : array) {
if (candidate.equalsIgnoreCase(s) {
return array[ROOT_WORD];
}
}
}
return candidate;
}

...

}

1
ответ дан 21 декабря 2011 в 02:12 Источник Поделиться

Вот мое мнение, это в принципе такой же, как другие. Примечания и предостережения, в порядке:


  • Я держу две карты, одна из основных => синонимов, один из синонимов => слово.

  • Добавлено состояния ошибки, если синоним зарегистрирован в двух разных праймериз.

  • Игнорировать, если синоним добавляется один основной, несколько раз.

  • Не обеспечивает синоним никогда не зарегистрированным в качестве основного, или наоборот--в реальности там должен быть решение о том, как справиться с этой ситуацией.

  • Тестирования кода, но не используется за пределами тестов был лишен.

  • Комментарии и проверка параметров раздели.

  • Я возвращать строку (присоединился с Викискладе Ланг), а не массив.

  • Одна ошибка слева в умышленные, ради забавы. Другие, вероятно, не намеренным.


public class Synonymizer {

private Map<String, Set<String>> wordSynonymsMap;

private Map<String, String> synonymWordMap;

public Synonymizer() {
wordSynonymsMap = new HashMap<String, Set<String>>();
synonymWordMap = new HashMap<String, String>();
}

public void addSynonyms(String word, String... synonyms) {
Set<String> wordSynonyms = getSynonyms(word);
for (String synonym : synonyms) {
wordSynonyms.add(synonym);
if (synonymWordMap.containsKey(synonym) && !synonymWordMap.get(synonym).equals(word)) {
throw new RuntimeException(String.format("'%s' already used as synonym for '%s'", synonym, synonymWordMap.get(synonym)));
}
synonymWordMap.put(synonym, word);
}
}

public Set<String> getSynonyms(String word) {
if (!wordSynonymsMap.containsKey(word)) {
wordSynonymsMap.put(word, new HashSet<String>());
}

return wordSynonymsMap.get(word);
}

public String normalizeString(String s) {
List<String> l = new ArrayList<String>();
for (String word : s.split(" ")) {
l.add(synonymize(word));
}

return StringUtils.join(l, " ");
}

public String synonymize(String word) {
if (wordSynonymsMap.containsKey(word)) {
return word;
}

String primary = synonymWordMap.get(word);
if (primary != null) {
return primary;
}

return word;
}

}

1
ответ дан 22 декабря 2011 в 04:12 Источник Поделиться