Основная проблемма рефакторинга.
Опытные инженеры (нетолько 'тире' программисты) говорят:
если что-то работает, то не трогай это!
Это идет вразрез с позицией Мартина Фаулера, который утверждает, что если что-то "пахнет",
то надо переделать.
ИМХО - здесь он прав:
"не тронь - не воняет".
А уж если завоняло, то как-то самому неприятно становится.
Все бы хорошо, но вот код
/**
* Formats decimal number (sum) down to cents
* @param number Number that is formatted
* @return Double formated number
*/
public static double format(final double number) {
return Double.parseDouble(String.format(new Locale("en"), "%04.2f",number));
}
из числа строку из строки в число - и все одной строкой. - "Запах" не очень приятный
А я человек чувствительный к такого рода "запахам".
Проблем возникающих в случае если делать хотябы так:
public static double format(final double number) {
return Math.round(number*100)/100;
}
я не нашел.
Но код писал явно человек знающий компьютер и языки программирования - а значит...
Ну человек разумный в конце концов. "А вдруг здесь какая-то хитрость" - подумал я.
На этом мысль застопорилось. Теперь с одной стороны "запах" - с другой "возможная хитрость".
Может кто подскажет как быть в таком случае?
Подписаться на:
Комментарии к сообщению (Atom)
7 комментариев:
Я думаю, чтобы выявить = хитрость это или запах, необходимо предоставить юнитест. Ждем.
Вот некоторое подобие юнит теста. плз:
import java.util.Locale;
public class TestTrickOrStink {
public static double originalFormat(final double number) {
return Double.parseDouble(String.format(new Locale("en"), "%04.2f",
number));
}
public static double refactoringFormat(final double number) {
return Math.round(number * 100.0) / 100.0;
}
public static boolean detect() {
Double[] testValues = new Double[] {
12.46565,
13.5352,
0.1234,
0.7564,
1.0,
32.0,
47.456,
10000.0099999999,
10000.00000000001
};
for (int i = 0; i < testValues.length; i++) {
Double test = testValues[i];
Double val1 = originalFormat(test);
Double val2 = refactoringFormat(test);
System.out.println("Take " + test
+ " and giving from originalFormat: " + val1
+ ", from refactoringFormat: " + val2);
if (!val1.equals(val2)) {
System.out.println("WOW!!! It is Different for " + test
+ " originalFormat give: " + val1 + " but "
+ " refactoringFormat give: " + val2);
return true;
}
}
return false;
}
public static void main(String[] args) {
System.out.println("it is: "+(detect()?"trick":"stink."))
}
}
Я думаю, что если посрать не сидя, а стоя. Запах от этого не изменится.
На счет принципиальной разницы
Я считаю, что программист, который может писать код, должен как можно больше отвязыватся от контекста его использования, ибо тем самым он исключает дублирование кода и что тоже немало важно дублирование результата разными алгоритмами. Да я согласен с тем, что если функция format будет использоваться толь 1 раз в месяц, день или минуту - то принципиальной разницы нет.
Но если эта функция "протащится" через даже небольшую иерархию объектов, и будет использоваться в каком-нибудь цикле, то обнаружить потерю производительности будет, не очень просто.
Другой аспект - этот код будет багом если в текущей локале например разделителем разрядов будет не точка а запятая, тогда преобразование parseDouble() будет выполнятся с текущей локалью и искать запятую как разделитель разрядов, а передоваться будет строка с разделителем -точкой.
Исходя из всего этого я делаю вывод, что такого рода привязки к контексту выполнения делают разницу принципиальной
Чувак просто не проходил щколу DecimalSeparator'а Ж)
Копаясь в CVS с mindgammer -ом нашли первый вариант этой функции вот он, плз.
/**
* Formats decimal numbers
*
* @param double number
*
* @return double formats number
*/
public static double format(double number)
{
DecimalFormat df = new DecimalFormat(".##");
try {
return df.parse(df.format(number)).doubleValue();
}
catch (ParseException pe) {
pe.printStackTrace();
return number;
}
}
Мы тут еще нашли.... Просто нет слов...
/**
* get date with timezone offset from calendar
* @param cal
* @return Date
*/
public static Date getDate(final Calendar cal) throws Exception {
final SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd HH:mm");
return df.parse(cal.get(Calendar.YEAR) + "-"
+ String.valueOf(cal.get(Calendar.MONTH) + 1) + "-"
+ cal.get(Calendar.DATE) + " " + cal.get(Calendar.HOUR_OF_DAY)
+ ":" + cal.get(Calendar.MINUTE));
}
Что делает этот метод? И в чем отличие от Calendar.getTime()?
Отправить комментарий