Your function is not really optimal because you will **ALWAYS** iterate until the end of your values array even if your number becomes zero! Which is not optimal at all.

In addition to that you didn't even check if the input number has a valid roman representation. You had to check that (even if the exercice assumes that the number has it already), you can do that easily with one line.

Here is an improvement of your function :

```
public static String intToRoman(int num){
if (num < 1 || num > 3999) return "";
StringBuilder result = new StringBuilder();
String[] roman = {"M", "CM", "D", "CD", "C", "XC", "L", "XL", "X", "IX", "V", "IV", "I"};
int[] values = {1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1 };
int i = 0;
//iterate until the number becomes zero, NO NEED to go until the last element in roman array
while (num >0) {
while ( num >= values[i]) {
num -= values[i];
result.append(roman[i]);
}
i++;
}
return result.toString();
}
```

As you see, you add just a simple line and you win an optimisation since the code will stop if the current number becomes zero (instead of doing additionnal iterations in the roman array).