From 8930e139b28ff165cca2d818199505a727106466 Mon Sep 17 00:00:00 2001
From: cgoIT <carsten.goetzinger@concedro.com>
Date: Sat, 8 Apr 2023 19:33:33 +0200
Subject: [PATCH] fix(roman-numeral-converter): input validation and feedback
 (#332)

* fix(roman-numeral-converter):  checks for valid input and conversion enhancements

Validates if numeral values are between 1 and 3999999.
Validates if a roman number is valid.

* fix(roman-numeral-converter): optimize logic for copy button

* fix(roman-numeral-converter): changes due to review
---
 .../roman-numeral-converter.service.test.ts   | 18 ++++----
 .../roman-numeral-converter.service.ts        | 13 +++++-
 .../roman-numeral-converter.vue               | 45 ++++++++++++++++---
 3 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/src/tools/roman-numeral-converter/roman-numeral-converter.service.test.ts b/src/tools/roman-numeral-converter/roman-numeral-converter.service.test.ts
index 21a747cf..5ec9dd47 100644
--- a/src/tools/roman-numeral-converter/roman-numeral-converter.service.test.ts
+++ b/src/tools/roman-numeral-converter/roman-numeral-converter.service.test.ts
@@ -13,14 +13,17 @@ describe('roman-numeral-converter', () => {
       expect(arabicToRoman(0.9)).toEqual('');
     });
 
+    it('should convert numbers greater than 3999 to empty string', () => {
+      expect(arabicToRoman(3999.1)).toEqual('');
+      expect(arabicToRoman(4000)).toEqual('');
+      expect(arabicToRoman(10000)).toEqual('');
+    });
+
     it('should convert floating points number to the lower integer in roman version', () => {
-      expect(arabicToRoman(-100)).toEqual('');
-      expect(arabicToRoman(-42)).toEqual('');
-      expect(arabicToRoman(-26)).toEqual('');
-      expect(arabicToRoman(-10)).toEqual('');
-      expect(arabicToRoman(0)).toEqual('');
-      expect(arabicToRoman(0.5)).toEqual('');
-      expect(arabicToRoman(0.9)).toEqual('');
+      expect(arabicToRoman(1.1)).toEqual('I');
+      expect(arabicToRoman(1.9)).toEqual('I');
+      expect(arabicToRoman(17.6)).toEqual('XVII');
+      expect(arabicToRoman(29.999)).toEqual('XXIX');
     });
 
     it('should convert positive integers to roman numbers', () => {
@@ -67,7 +70,6 @@ describe('roman-numeral-converter', () => {
       expect(arabicToRoman(999)).toEqual('CMXCIX');
       expect(arabicToRoman(1000)).toEqual('M');
       expect(arabicToRoman(2000)).toEqual('MM');
-      expect(arabicToRoman(9000)).toEqual('MMMMMMMMM');
     });
   });
 });
diff --git a/src/tools/roman-numeral-converter/roman-numeral-converter.service.ts b/src/tools/roman-numeral-converter/roman-numeral-converter.service.ts
index df2408a0..98afec67 100644
--- a/src/tools/roman-numeral-converter/roman-numeral-converter.service.ts
+++ b/src/tools/roman-numeral-converter/roman-numeral-converter.service.ts
@@ -1,5 +1,7 @@
+export const MIN_ARABIC_TO_ROMAN = 1;
+export const MAX_ARABIC_TO_ROMAN = 3999;
 export function arabicToRoman(num: number) {
-  if (num < 1) return '';
+  if (num < MIN_ARABIC_TO_ROMAN || num > MAX_ARABIC_TO_ROMAN) return '';
 
   const lookup: { [key: string]: number } = {
     M: 1000,
@@ -26,7 +28,16 @@ export function arabicToRoman(num: number) {
   return roman;
 }
 
+const ROMAN_NUMBER_REGEX = new RegExp(/^M{0,3}(CM|CD|D?C{0,3})(XC|XL|L?X{0,3})(IX|IV|V?I{0,3})$/);
+
+export function isValidRomanNumber(romanNumber: string) {
+  return ROMAN_NUMBER_REGEX.test(romanNumber);
+}
+
 export function romanToArabic(s: string) {
+  if (!isValidRomanNumber(s)) {
+    return null;
+  }
   const map: { [key: string]: number } = { I: 1, V: 5, X: 10, L: 50, C: 100, D: 500, M: 1000 };
   return [...s].reduce((r, c, i, s) => (map[s[i + 1]] > map[c] ? r - map[c] : r + map[c]), 0);
 }
diff --git a/src/tools/roman-numeral-converter/roman-numeral-converter.vue b/src/tools/roman-numeral-converter/roman-numeral-converter.vue
index c55380d0..609b46c4 100644
--- a/src/tools/roman-numeral-converter/roman-numeral-converter.vue
+++ b/src/tools/roman-numeral-converter/roman-numeral-converter.vue
@@ -2,21 +2,29 @@
   <div>
     <n-card title="Arabic to roman">
       <n-space align="center" justify="space-between">
-        <n-input-number v-model:value="inputNumeral" :min="1" style="width: 200px" :show-button="false" />
+        <n-form-item v-bind="validationNumeral">
+          <n-input-number v-model:value="inputNumeral" :min="1" style="width: 200px" :show-button="false" />
+        </n-form-item>
         <div class="result">
           {{ outputRoman }}
         </div>
-        <n-button secondary autofocus @click="copyRoman"> Copy </n-button>
+        <n-button secondary autofocus :disabled="validationNumeral.validationStatus === 'error'" @click="copyRoman">
+          Copy
+        </n-button>
       </n-space>
     </n-card>
     <br />
     <n-card title="Roman to arabic">
       <n-space align="center" justify="space-between">
-        <n-input v-model:value="inputRoman" style="width: 200px" />
+        <n-form-item v-bind="validationRoman">
+          <n-input v-model:value="inputRoman" style="width: 200px" />
+        </n-form-item>
         <div class="result">
           {{ outputNumeral }}
         </div>
-        <n-button secondary autofocus @click="copyArabic"> Copy </n-button>
+        <n-button secondary autofocus :disabled="validationRoman.validationStatus === 'error'" @click="copyArabic">
+          Copy
+        </n-button>
       </n-space>
     </n-card>
   </div>
@@ -25,14 +33,41 @@
 <script setup lang="ts">
 import { useCopy } from '@/composable/copy';
 import { ref, computed } from 'vue';
-import { arabicToRoman, romanToArabic } from './roman-numeral-converter.service';
+import { useValidation } from '@/composable/validation';
+import {
+  arabicToRoman,
+  romanToArabic,
+  MAX_ARABIC_TO_ROMAN,
+  MIN_ARABIC_TO_ROMAN,
+  isValidRomanNumber,
+} from './roman-numeral-converter.service';
 
 const inputNumeral = ref(42);
 const outputRoman = computed(() => arabicToRoman(inputNumeral.value));
 
+const { attrs: validationNumeral } = useValidation({
+  source: inputNumeral,
+  rules: [
+    {
+      validator: (value) => value >= MIN_ARABIC_TO_ROMAN && value <= MAX_ARABIC_TO_ROMAN,
+      message: `We can only convert numbers between ${MIN_ARABIC_TO_ROMAN.toLocaleString()} and ${MAX_ARABIC_TO_ROMAN.toLocaleString()}`,
+    },
+  ],
+});
+
 const inputRoman = ref('XLII');
 const outputNumeral = computed(() => romanToArabic(inputRoman.value));
 
+const { attrs: validationRoman } = useValidation({
+  source: inputRoman,
+  rules: [
+    {
+      validator: (value) => isValidRomanNumber(value),
+      message: `The input you entered is not a valid roman number`,
+    },
+  ],
+});
+
 const { copy: copyRoman } = useCopy({ source: outputRoman, text: 'Roman number copied to the clipboard' });
 const { copy: copyArabic } = useCopy({ source: outputNumeral, text: 'Arabic number copied to the clipboard' });
 </script>