Convert integer to Roman numeral string

Question

This is my 1st time developing PHP code and I am only 13 years old. I have experience in other programming languages like lua, python, and c++.

This is a simple Number to Roman Numeral code I made for my mom because she had a hard time making herself one. Below is my code. Don't mind the names of the variables.

<?php
 $NumberRomanNumeral = array(
 1 => array(
 "RomanNumeral" => "I",
 "NearestNum" => null
 ),
 5 => array(
 "RomanNumeral" => "V",
 "NearestNum" => 4
 ),
 10 => array(
 "RomanNumeral" => "X",
 "NearestNum" => 9
 ),
 50 => array(
 "RomanNumeral" => "L",
 "NearestNum" => 40
 ),
 100 => array(
 "RomanNumeral" => "C",
 "NearestNum" => 90
 ),
 500 => array(
 "RomanNumeral" => "D",
 "NearestNum" => 400
 ),
 1000 => array(
 "RomanNumeral" => "M",
 "NearestNum" => 900
 ),
 );
 
 function separateNumberIntoUnits($n) {
 $separated = str_split($n,1);
 for ($i = 0;$i <= count($separated)-1; $i++){
 $currentNum = $separated[$i];
 $length = count($separated) - $i;
 $separated[$i] = str_pad($currentNum, $length, "0");
 }
 return $separated;
 } 
 
 function getClosestNum($numToFind){
 //echo($numToFind);
 global $NumberRomanNumeral;
 $closestNum = null;
 $foundKey = null;
 $closestMag = null;
 $differenceArray = array();
 $numLeft = null;
 if ($numToFind == 0) {
 return null;
 }
 foreach($NumberRomanNumeral as $num => $data){
 array_push($differenceArray, $data["NearestNum"]);
 }
 foreach($NumberRomanNumeral as $num => $data){
 $currentIndex = array_search($num,array_keys($NumberRomanNumeral));
 $romanNumeral = $data["RomanNumeral"];
 $differenceToSum = $data["NearestNum"];
 $previousDifferenceToSum = null;
 if ($num != 1){
 $previousDifferenceToSum = $differenceArray[$currentIndex-1];
 }
 $mag = $num - $numToFind;
 if ($mag < 0){
 $mag *= -1;
 }
 if ($numToFind == $num){
 $closestNum = $num;
 $foundKey = $romanNumeral;
 break;
 }elseif($numToFind == $differenceToSum){
 $closestNum = $num;
 $foundKey = $romanNumeral;
 break;
 }
 if ($closestMag == null){
 $closestNum = $num;
 $closestMag = $mag;
 $foundKey = $romanNumeral;
 } else {
 if ($mag < $closestMag && $mag > $previousDifferenceToSum) {
 $closestNum = $num;
 $closestMag = $mag;
 $foundKey = $romanNumeral;
 }
 }
 }
 $numLeft = $numToFind - $closestNum;
 if ($numLeft < 0){
 $numLeft *= -1;
 }
 return array($closestNum, $foundKey, $numLeft);
 }
 
 function num2RomanNumeral($num, &$string){
 if ($num != null || $num != 0){
 $numArray = separateNumberIntoUnits($num);
 foreach($numArray as $splitNum){
 $data = getClosestNum($splitNum);
 if ($data != null){
 $closestNumber = $data[0]; 
 $foundRomanNumeral = $data[1];
 $numLeft = $data[2];
 if ($splitNum > $closestNumber){
 $string = $string.$foundRomanNumeral;
 if ($numLeft > 0){
 num2RomanNumeral($numLeft, $string);
 }
 }elseif($splitNum < $closestNumber){
 if ($numLeft > 0){
 num2RomanNumeral($numLeft, $string);
 }
 $string = $string.$foundRomanNumeral;
 }else{
 if ($numLeft > 0){
 num2RomanNumeral($numLeft, $string);
 }
 $string = $string.$foundRomanNumeral;
 }
 
 }
 }
 }
 }
 
 $romanNumeralText = "";
 num2RomanNumeral(4212, $romanNumeralText);
 echo $romanNumeralText;
?>

I would appreciate if you give some tips and some things that would make the code better.

• mickmackusa: Relevant reading: Numbers to Roman Numbers with php

Answer 1

For a beginner it seems fine. There are some improvements that can be made. The points below are analysis of the given code.

Obviously there are examples of simpler implementations to convert numbers to Roman numeral format - e.g. see answers to Numbers to Roman Numbers with php on Stack Overflow. While I wouldn't expect a beginner to write a succinct and optimized function, they are out there for the reader to compare.

Avoid Global variables

The first thing I spot is this line near the beginning of getClosestNum()

global $NumberRomanNumeral;

Global variables have more negative aspects than positives. Since that variable is never re-assigned it can be declared as a constant instead of a variable. Then there is no need to reference a global variable.

Coding Style can be changed to be idiomatic

I would not expect a beginner to write code that always adheres to a standard coding style, but there are recommended styles that many PHP developers write code to be inline with. One popular style guide is PSR-12. It has many conventions for readability and robust code. There are 54 errors and 1 warning for the code above. Many of the errors concern spacing between operators, tokens, etc.

For example, instead of:

}elseif($numToFind == $differenceToSum){

Idiomatic PHP code would be spaced like this:

} elseif ($numToFind == $differenceToSum) {

Array syntax can be simplified

There isn't anything wrong with using array() but as of the time of writing, there is currently active support for versions 8.0 and 8.1 of PHP, and since PHP 5.4 arrays can be declared with short array syntax (PHP 5.4).

So lines like:

$differenceArray = array();

can be simplified like this:

$differenceArray = [];

Variable gets overwritten

The seventh line of getClosestNum() is:

$numLeft = null;

Then towards the end of the function, that same variable is assigned with the difference of two other variables.

$numLeft = $numToFind - $closestNum;

This makes the first assignment (to null) superfluous. That line can be removed.

Extra indentation level can be eliminated

Near the end of getClosestNum() are these blocks of code:

 if ($closestMag == null) {
 $closestNum = $num;
 $closestMag = $mag;
 $foundKey = $romanNumeral;
 } else {
 if ($mag < $closestMag && $mag > $previousDifferenceToSum) {
 $closestNum = $num;
 $closestMag = $mag;
 $foundKey = $romanNumeral;
 }
 }

The second if (within the else) can be changed to an elseif:

 if ($closestMag == null) {
 $closestNum = $num;
 $closestMag = $mag;
 $foundKey = $romanNumeral;
 } elseif ($mag < $closestMag && $mag > $previousDifferenceToSum) {
 $closestNum = $num;
 $closestMag = $mag;
 $foundKey = $romanNumeral;
 }

This allows for one less indentation level for the lines within the else block.