Говнокод #11562 — Java — Говнокод.ру

Говнокод: по колено в коде.

Нашли или выдавили из себя код, который нельзя назвать нормальным, на который без улыбки не взглянешь? Не торопитесь его удалять или рефакторить, — запостите его на говнокод.ру, посмеёмся вместе!

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

+72

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 Августа 2012

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

tir 10.08.2012 08:17 # +1

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

Отрефакторил. Хотя 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 10.08.2012 09:26 # +2
  Твою мать, нужна превьюшка!
  
  // литералы в константы 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 10.08.2012 10:04 # +2
    
    с ходу не смог разобраться. копипаста ушла, читаемости не особо добавилось
    Ответить
tir 10.08.2012 10:05 # −1
мой вариант
```
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 10.08.2012 10:47 # +1
  
  В вашем варианте в трех местах используется конструкция if - else и в одном месте тернарный оператор, по сравнению с тем, что предложил я (один раз). Используется проверка на null, которую я вообще убрал за ненадобностью. Не думаю, что это легче разобрать, а главное сопровождать. Логики в коде гораздо больше.
  Ответить
  - tir 10.08.2012 11:13 # 0
    
    где же больше логики?
    Ответить
  - tir 10.08.2012 11:16 # 0
    
    кстати, ваш вариант работает неправильно когда мы просим "headerTitle.suffix" если он оказывается пустой.
    Ответить
    - guest 10.08.2012 11:36 # 0
      
      Да, вы правы. Я был невнимателен. Ничего не придумал как добавить метод аналогичный вашему:
      
      private void appendSuffix(StringBuilder sb) { String suffix = getMessageOrEmptyString(HEADER_SUFFIX_PROP); if (!suffix.isEmpty()) { title.append(" "); title.append(suffix); } }
      
      Еще одна конструкция if.
      Ответить
      - tir 10.08.2012 11:46 # 0
        
        я не это имел ввиду. Это, кстати, моя инициатива - в исходном коде этой логики нет.
        
        Я имел ввиду ситуацию getMessage("headerTitle.suffix", false)
        Ваш код вернет дефолтный заголовок, если значение окажется пустым, а исходный код вернет пустое значение.
        Ответить
- roman-kashitsyn 10.08.2012 11:01 # 0
  
  частично лучше (правда, черезчур громоздко), но мне всё равно не нравится идея смешивать ответственность за получение сообщения с логикой форматирования этого сообщения. И не важно, приватный метод или нет.
  Ответить
  - tir 10.08.2012 11:08 # 0
    
    можно переименовать getMessage в getFormattedMessage. Реально сообщение получается через getSourceMessage.
    Ответить
mIwxJAl 25.08.2021 07:38 # 0

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

Говнокод: по колено в коде.

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

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

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