res3ukt
06.Сентябрь.2020 14:20:44
#1
задание на codewars
Modern Roman numerals are written by expressing each digit separately starting with the left most digit and skipping any digit with a value of zero. In Roman numerals 1990 is rendered: 1000=M, 900=CM, 90=XC; resulting in MCMXC. 2008 is written as 2000=MM, 8=VIII; or MMVIII. 1666 uses each Roman symbol in descending order: MDCLX
public static String solution(int n) {
map.put(1, "I");
map.put(5, "V");
map.put(10, "X");
map.put(50, "L");
map.put(100, "C");
map.put(500, "D");
map.put(1000, "M");
numGen(n, map);
while (!numbers.isEmpty()) {
romeNumCreator(numbers.poll());
}
return sb.toString();
}
static Queue<Integer> numGen(int n, Map<Integer, String> map) {
//int[] discharges = {1, 10, 100, 1000};
String s = String.valueOf(n);
int length = s.length();
do {
if (map.containsKey(n)) {
numbers.add(n);
break;
}
int temp = n / DISCHARGES[length - 1];
if (temp == 0) {
length--;
continue;
}
n %= DISCHARGES[length - 1];
int num = temp * DISCHARGES[length - 1];
numbers.add(num);
length--;
} while (length > 0);
return numbers;
}
static void romeNumCreator(int num) {
int n = num;
if (map.containsKey(n)) {
sb.append(map.get(n));
return;
}
int length = String.valueOf(n).length();
int count = n / DISCHARGES[length - 1];
if (count <= MAX_REPEAT) {
while (count > 0) {
sb.append(map.get(DISCHARGES[length - 1]));
count--;
}
} else if (map.containsKey(n + DISCHARGES[length - 1])) {
sb.append(map.get(DISCHARGES[length - 1])).append(map.get(n + DISCHARGES[length - 1]));
} else {
int sum = 0;
int res = n;
do {
res -= DISCHARGES[length - 1];
sum += DISCHARGES[length - 1];
} while (!map.containsKey(res));
sb.append(map.get(res));
romeNumCreator(sum);
}
}
}
Вот что мне выдает test с codewars
shouldConvertToRoman
solution(4) should equal to IV expected:<I[]V> but was:<I[I]V>
AlexP
06.Сентябрь.2020 14:53:49
#2
Тут видимо написано, что вывело IIV вместо IV.
ЗЫ при вставке кода на форум надо было на пустую строку перейти перед нажатием кнопки Код )
res3ukt
06.Сентябрь.2020 14:54:39
#3
но у меня вывод верный же
AlexP
06.Сентябрь.2020 14:56:55
#4
В смысле верный? Как проверяли?
Оно же пишет, что должно получиться IV (при вводе 4), а вывело IIV.
res3ukt
06.Сентябрь.2020 15:05:00
#6
откуда берется II я не пойму
res3ukt
06.Сентябрь.2020 16:41:49
#8
давал на вход 4 и на выходе получал iv
AlexP
06.Сентябрь.2020 17:00:51
#9
Скиньте весь код, в первом сообщении начало пропущено.
А это где на codewars? Тут я что-то не вижу Джавы https://www.codewars.com/kata/51b6249c4612257ac0000005/
res3ukt
06.Сентябрь.2020 17:14:59
#10
public class Main {
static final int MAX_REPEAT = 3;
static final int[] DISCHARGES = {1, 10, 100, 1000};
static Queue<Integer> numbers = new ArrayDeque<>();
static StringBuilder sb = new StringBuilder();
static Map<Integer, String> map = new HashMap<>();
public static void main(String[] args) {
//Map<Integer, Character> map = new HashMap<>();
/*map.put(1, 'I');
map.put(5, 'V');
map.put(10, 'X');
map.put(50, 'L');
map.put(100, 'C');
map.put(500, 'D');
map.put(1000, 'M');
numGen(3645, map);
while (!numbers.isEmpty()) {
romeNumCreator(numbers.poll());
}
System.out.println(sb.toString());
*/
System.out.println(solution(6));
}
public static String solution(int n) {
map.put(1, "I");
map.put(5, "V");
map.put(10, "X");
map.put(50, "L");
map.put(100, "C");
map.put(500, "D");
map.put(1000, "M");
numGen(n, map);
while (!numbers.isEmpty()) {
romeNumCreator(numbers.poll());
}
return sb.toString();
}
static Queue<Integer> numGen(int n, Map<Integer, String> map) {
//int[] discharges = {1, 10, 100, 1000};
String s = String.valueOf(n);
int length = s.length();
do {
if (map.containsKey(n)) {
numbers.add(n);
break;
}
int temp = n / DISCHARGES[length - 1];
if (temp == 0) {
length--;
continue;
}
n %= DISCHARGES[length - 1];
int num = temp * DISCHARGES[length - 1];
numbers.add(num);
length--;
} while (length > 0);
return numbers;
}
static void romeNumCreator(int num) {
int n = num;
if (map.containsKey(n)) {
sb.append(map.get(n));
return;
}
int length = String.valueOf(n).length();
int count = n / DISCHARGES[length - 1];
if (count <= MAX_REPEAT) {
while (count > 0) {
sb.append(map.get(DISCHARGES[length - 1]));
count--;
}
} else if (map.containsKey(n + DISCHARGES[length - 1])) {
sb.append(map.get(DISCHARGES[length - 1])).append(map.get(n + DISCHARGES[length - 1]));
} else {
int sum = 0;
int res = n;
do {
res -= DISCHARGES[length - 1];
sum += DISCHARGES[length - 1];
} while (!map.containsKey(res));
sb.append(map.get(res));
romeNumCreator(sum);
}
}
}
AlexP
06.Сентябрь.2020 17:29:53
#12
А так?
System.out.println(solution(1));
System.out.println(solution(4));
Это пример того, почему по возможности нужно избегать любого глобального состояния.
Чистые функции рулят.
https://ru.wikipedia.org/wiki/Ссылочная_прозрачность https://en.wikipedia.org/wiki/Immutable_object
res3ukt
06.Сентябрь.2020 18:16:44
#13
Спасибо вам!!!
import java.util.ArrayDeque;
import java.util.HashMap;
import java.util.Map;
import java.util.Queue;
public class Main {
static final int MAX_REPEAT = 3;
static final int[] DISCHARGES = {1, 10, 100, 1000};
static Queue<Integer> numbers = new ArrayDeque<>();
static StringBuilder sb;
static Map<Integer, String> map = new HashMap<>();
public static void main(String[] args) {
//Map<Integer, Character> map = new HashMap<>();
/*map.put(1, 'I');
map.put(5, 'V');
map.put(10, 'X');
map.put(50, 'L');
map.put(100, 'C');
map.put(500, 'D');
map.put(1000, 'M');
numGen(3645, map);
while (!numbers.isEmpty()) {
romeNumCreator(numbers.poll());
}
System.out.println(sb.toString());
*/
System.out.println(solution(1));
System.out.println(solution(4));
}
public static String solution(int n) {
sb = new StringBuilder();
map.put(1, "I");
map.put(5, "V");
map.put(10, "X");
map.put(50, "L");
map.put(100, "C");
map.put(500, "D");
map.put(1000, "M");
numGen(n, map);
while (!numbers.isEmpty()) {
romeNumCreator(numbers.poll());
}
return sb.toString();
}
static Queue<Integer> numGen(int n, Map<Integer, String> map) {
//int[] discharges = {1, 10, 100, 1000};
String s = String.valueOf(n);
int length = s.length();
do {
if (map.containsKey(n)) {
numbers.add(n);
break;
}
int temp = n / DISCHARGES[length - 1];
if (temp == 0) {
length--;
continue;
}
n %= DISCHARGES[length - 1];
int num = temp * DISCHARGES[length - 1];
numbers.add(num);
length--;
} while (length > 0);
return numbers;
}
static void romeNumCreator(int num) {
int n = num;
if (map.containsKey(n)) {
sb.append(map.get(n));
return;
}
int length = String.valueOf(n).length();
int count = n / DISCHARGES[length - 1];
if (count <= MAX_REPEAT) {
while (count > 0) {
sb.append(map.get(DISCHARGES[length - 1]));
count--;
}
} else if (map.containsKey(n + DISCHARGES[length - 1])) {
sb.append(map.get(DISCHARGES[length - 1])).append(map.get(n + DISCHARGES[length - 1]));
} else {
int sum = 0;
int res = n;
do {
res -= DISCHARGES[length - 1];
sum += DISCHARGES[length - 1];
} while (!map.containsKey(res));
sb.append(map.get(res));
romeNumCreator(sum);
}
}
}
критикуйте, помоему так как я исправил не пишется, это не красиво))
AlexP
06.Сентябрь.2020 18:29:37
#14
Можно просто это всё сделать переменными внутри функций
И заодно использовать тут возвращаемое значение
res3ukt:
numGen(n, map);
Queue<Integer> numbers = numGen...
(то есть внутри numGen создавать эту Queue)
И еще неплохо бы улучшить имена.
https://ru.hexlet.io/blog/posts/naming-in-programming
res3ukt
06.Сентябрь.2020 18:33:06
#15
У меня просто рекурсия внутри romeNumCreator
AlexP
06.Сентябрь.2020 18:44:17
#16
sb можно просто передавать параметром, это объект, они передаются по ссылке.
map можно оставить глобальным.неизменяемым (например после создания в конструкторе класса, сделав всё не static).
