Говнокод

Jawa / Говнокод #11562

private String getMessage(String prop, boolean suffixEnabled) {
        String title = null;
        if (prop.equals("headerTitle.suffix")) {
            try {
                title = messageSource.getMessage("headerTitle.suffix", null, locale);
            } catch (NoSuchMessageException e) {
                //e.printStackTrace();
            }
            if (title == null)
                title = "";
        } else {
            try {
                title = messageSource.getMessage(prop, null, locale);
                if (suffixEnabled)
                    title += " " + messageSource.getMessage("headerTitle.suffix", null, locale);
            } catch (NoSuchMessageException e) {
                //e.printStackTrace();
            }
            if (title == null) {
                try {
                    title = messageSource.getMessage("headerTitle.default", null, locale);
                } catch (NoSuchMessageException ex) {
                    title = "";
                }
            }

        }
        return title;
    }

Запостил: welvet, 09.08.2012 (Updated 25.03.2018)

Комментарии (15) RSS

tir 14 лет ago # 0

что не так?
Ответить
- roman-kashitsyn 14 лет ago # 0 ⇈
  
  1. Сама идея добавлять пробел к сообщению в зависимости от флажка. ProblemFactory не одобряе.
  2. Слишком много копипаста.
  Ответить
  - tir 14 лет ago # 0 ⇈
    
    с копипастой соглашусь
    
    с флагом 50/50. метод приватный, значит используется для какого-то юзкейса внутри класса скорее всего.
    Ответить
guest 14 лет ago # 0

Отрефакторил. Хотя StringBuilder можно было и опустить, использовав простую конкатенацию с пробелом (не критично).

[code=java]
// литералы в константы
private static final String HEADER_SUFFIX_PROP = "headerTitle.suffix";
private static final String DEFAULT_TITLE_PROP = "headerTitle.default";

//.....

private String getMessage(String prop, boolean suffixEnabled) {
StringBuilder title = new StringBuilder(getMessageOrEmptyString(pr op));

if (title.isEmpty()) {
title.append(getMessageOrEmptyString(DEF AULT_TITLE_PROP));
} else if (suffuxEnabled && !HEADER_SUFFIX_PROP.equals(prop)) {
title.append(" ");
title.append(getMessageOrEmptyString(HEA DER_SUFFIX_PROP));
}
return title.toString();
}

private String getMessageOrEmptyString(String prop) {
try {
return messageSource.getMessage(prop, null, locale);
} catch (NoSuchMessageException e) {
// добавить warning лог о несуществующем мессадже
return "";
}
}
[code]
Ответить
- guest 14 лет ago # 0 ⇈
  Твою мать, нужна превьюшка!
```
// литералы в константы
private static final String HEADER_SUFFIX_PROP = "headerTitle.suffix";
private static final String DEFAULT_TITLE_PROP = "headerTitle.default";

private String getMessage(String prop, boolean suffixEnabled) {
    StringBuilder title = new StringBuilder(getMessageOrEmptyString(prop));

    if (title.isEmpty()) {
        title.append(getMessageOrEmptyString(DEFAULT_TITLE_PROP));
    } else if (suffuxEnabled && !HEADER_SUFFIX_PROP.equals(prop)) {       
        title.append(" ");
        title.append(getMessageOrEmptyString(HEADER_SUFFIX_PROP));
    }
    return title.toString();
}

private String getMessageOrEmptyString(String prop) {
    try {
        return messageSource.getMessage(prop, null, locale);
    } catch (NoSuchMessageException e) {
        // добавить warning лог о несуществующем мессадже
        return "";
    }
}
```
  Ответить
  - tir 14 лет ago # 0 ⇈
    
    с ходу не смог разобраться. копипаста ушла, читаемости не особо добавилось
    Ответить
tir 14 лет ago # 0
мой вариант
```
private static final String HEADER_SUFFIX = "headerTitle.suffix";
	private static final String HEADER_DEFAULT = "headerTitle.default";
	
	private String getMessage(String prop, boolean suffixEnabled) {
        if (isSuffix(prop)) {
        	return getSuffix();
        } else {
        	String title = getSourceMessage(prop, true);
        	if (title == null) {
        		return getDefaultHeader();
    		} else {
    			return suffixEnabled ? withSuffix(title) : title;
    		}
        }
    }
	
	private boolean isSuffix(String prop) {
		return HEADER_SUFFIX.equals(prop);
	}
	
	private String getSuffix() {
		return getSourceMessage(HEADER_SUFFIX, false);
	}
	
	private String withSuffix(String text) {
		String suffix = getSuffix();
		if (suffix.isEmpty()) {
			return text;
		} else {
			return text + " " + suffix;
		}
	}
	
	private String getDefaultHeader() {
		return getSourceMessage(HEADER_DEFAULT, false);
	}
	
	private String getSourceMessage(String prop, boolean nullIfAbsent) {
        try {
            return messageSource.getMessage(prop, null, locale);
        } catch (NoSuchMessageException ex) {
            return nullIfAbsent ? null : "";
        }		
	}
```
Ответить
- guest 14 лет ago # 0 ⇈
  
  В вашем варианте в трех местах используется конструкция if - else и в одном месте тернарный оператор, по сравнению с тем, что предложил я (один раз). Используется проверка на null, которую я вообще убрал за ненадобностью. Не думаю, что это легче разобрать, а главное сопровождать. Логики в коде гораздо больше.
  Ответить
  - tir 14 лет ago # 0 ⇈
    
    где же больше логики?
    Ответить
  - tir 14 лет ago # 0 ⇈
    
    кстати, ваш вариант работает неправильно когда мы просим "headerTitle.suffix" если он оказывается пустой.
    Ответить
    - guest 14 лет ago # 0 ⇈
      
      Да, вы правы. Я был невнимателен. Ничего не придумал как добавить метод аналогичный вашему:
      
      private void appendSuffix(StringBuilder sb) { String suffix = getMessageOrEmptyString(HEADER_SUFFIX_PROP); if (!suffix.isEmpty()) { title.append(" "); title.append(suffix); } }
      
      Еще одна конструкция if.
      Ответить
      - tir 14 лет ago # 0 ⇈
        
        я не это имел ввиду. Это, кстати, моя инициатива - в исходном коде этой логики нет.
        
        Я имел ввиду ситуацию getMessage("headerTitle.suffix", false)
        Ваш код вернет дефолтный заголовок, если значение окажется пустым, а исходный код вернет пустое значение.
        Ответить
- roman-kashitsyn 14 лет ago # 0 ⇈
  
  частично лучше (правда, черезчур громоздко), но мне всё равно не нравится идея смешивать ответственность за получение сообщения с логикой форматирования этого сообщения. И не важно, приватный метод или нет.
  Ответить
  - tir 14 лет ago # 0 ⇈
    
    можно переименовать getMessage в getFormattedMessage. Реально сообщение получается через getSourceMessage.
    Ответить
mIwxJAl 5 лет ago # 0

- Ты мой герой, но я ещё выясню, чем ты шантажировал Димошу, - она с какой-то гордостью произнесла это имя.
Ответить

Добавить комментарий Отменить ответ

Из-за тебя ушел bormand, guest!

А не использовать ли нам bbcode?