#776 Fix PR suggestions.

Author: yruslan

cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/encoding/BCDNumberEncoders.scala

@@ -22,7 +22,9 @@ object BCDNumberEncoders {
   /**
     * Encode a number as a binary encoded decimal (BCD) aka COMP-3 format to an array of bytes.
     *
-    * The length of the output array is determined by the formula: (precision + 1) / 2.
+    * Output length (bytes):
+    * - With mandatory sign nibble (signed or unsigned): ceil((precision + 1) / 2)
+    * - Unsigned without sign nibble: ceil(precision / 2).
     *
     * @param number              The number to encode.
     * @param precision           Total number of digits in the number.
@@ -55,27 +57,25 @@ object BCDNumberEncoders {
       return bytes
     }
 
-    val integralNumberStr = if (scaleFactor - scale == 0)
-      number.setScale(0, RoundingMode.HALF_DOWN).toString
-    else
-      number.movePointLeft(scaleFactor - scale).setScale(0, RoundingMode.HALF_DOWN).toString
+    val shift = scaleFactor - scale
+    val shifted = if (shift == 0) number else number.movePointLeft(shift)
 
-    val isNegative = integralNumberStr.startsWith("-")
-    val digitsOnly = integralNumberStr.stripPrefix("-").stripPrefix("+")
+    val isNegative = number.signum() < 0
+    val digitsOnly = shifted.abs().setScale(0, RoundingMode.HALF_DOWN).toPlainString
 
     if (isNegative && (!signed || !mandatorySignNibble)) {
       return bytes
     }
 
+    if (digitsOnly.length > precision || scale < 0)
+      return bytes
+
     val signNibble: Byte =  if (signed) {
       if (isNegative) 0x0D else 0x0C
     } else {
       0x0F
     }
 
-    if (digitsOnly.length > precision)
-      return bytes
-
     val padded = if (mandatorySignNibble) {
       if (digitsOnly.length == totalDigits - 1)
         digitsOnly + "0"

cobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/encoding/BCDNumberEncodersSuite.scala

@@ -29,6 +29,13 @@ class BCDNumberEncodersSuite extends AnyWordSpec {
         checkExpected(actual, expected)
       }
 
+      "encode a number with an even precision" in  {
+        val expected = Array[Byte](0x01, 0x23, 0x4C)
+        val actual = BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(1234), 4, 0, 0, signed = true, mandatorySignNibble = true)
+
+        checkExpected(actual, expected)
+      }
+
       "encode a small number" in  {
         val expected = Array[Byte](0x00, 0x00, 0x5C)
         val actual = BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(5), 5, 0, 0, signed = true, mandatorySignNibble = true)
@@ -64,6 +71,13 @@ class BCDNumberEncodersSuite extends AnyWordSpec {
         checkExpected(actual, expected)
       }
 
+      "encode a number without sign nibble with an even precision" in  {
+        val expected = Array[Byte](0x12, 0x34)
+        val actual = BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(1234), 4, 0, 0, signed = true, mandatorySignNibble = false)
+
+        checkExpected(actual, expected)
+      }
+
       "encode a too big number" in  {
         val expected = Array[Byte](0x00, 0x00, 0x00)
         val actual = BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(123456), 5, 0, 0, signed = false, mandatorySignNibble = false)
@@ -78,6 +92,13 @@ class BCDNumberEncodersSuite extends AnyWordSpec {
         checkExpected(actual, expected)
       }
 
+      "encode a number with nbegative scale" in  {
+        val expected = Array[Byte](0x00, 0x00, 0x00)
+        val actual = BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(12345), 5, -1, 0, signed = false, mandatorySignNibble = false)
+
+        checkExpected(actual, expected)
+      }
+
       "attempt to encode a negative number without sign nibble" in  {
         val expected = Array[Byte](0x00, 0x00, 0x00)
         val actual = BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(-12345), 5, 0, 0, signed = false, mandatorySignNibble = false)
@@ -105,6 +126,10 @@ class BCDNumberEncodersSuite extends AnyWordSpec {
 
         checkExpected(actual, expected)
       }
+
+      "attempt to encode a number with zero prexision" in  {
+        assertThrows[IllegalArgumentException](BCDNumberEncoders.encodeBCDNumber(new java.math.BigDecimal(12345), 0, 0, 0, signed = true, mandatorySignNibble = true))
+      }
     }
 
     "decimal number" when {

spark-cobol/src/main/scala/za/co/absa/cobrix/spark/cobol/writer/BasicRecordCombiner.scala

@@ -19,12 +19,10 @@ package za.co.absa.cobrix.spark.cobol.writer
 import org.apache.spark.rdd.RDD
 import org.apache.spark.sql.DataFrame
 import za.co.absa.cobrix.cobol.parser.Copybook
+import za.co.absa.cobrix.cobol.parser.ast.datatype.{Decimal, Integral}
 import za.co.absa.cobrix.cobol.parser.ast.{Group, Primitive, Statement}
 import za.co.absa.cobrix.cobol.reader.parameters.ReaderParameters
 import za.co.absa.cobrix.cobol.reader.schema.CobolSchema
-import za.co.absa.cobrix.cobol.parser.ast.datatype.{AlphaNumeric, COMP3, COMP3U, CobolType, Decimal, Integral}
-
-import java.io.{ByteArrayOutputStream, ObjectOutputStream}
 
 class BasicRecordCombiner extends RecordCombiner {
 
@@ -34,8 +32,8 @@ class BasicRecordCombiner extends RecordCombiner {
     val ast = getAst(cobolSchema)
     val copybookFields = ast.children.filter {
       case p: Primitive => !p.isFiller
-      case g: Group => !g.isFiller
-      case _ => true
+      case g: Group     => !g.isFiller
+      case _            => true
     }
 
     validateSchema(df, copybookFields.toSeq)
@@ -123,8 +121,8 @@ object BasicRecordCombiner {
 
     val usage = field.dataType match {
       case dt: Integral => dt.compact.map(_.toString).getOrElse("USAGE IS DISPLAY")
-      case dt: Decimal => dt.compact.map(_.toString).getOrElse("USAGE IS DISPLAY")
-      case _ => ""
+      case dt: Decimal  => dt.compact.map(_.toString).getOrElse("USAGE IS DISPLAY")
+      case _            => ""
     }
 
     s"$pic $usage".trim