From c20dc2cd0088176db2f6a942ea3bd1e9c1d41a33 Mon Sep 17 00:00:00 2001
From: Ferenc Fabian <qwer.kocka@gmail.com>
Date: Fri, 1 Sep 2023 17:53:19 +0200
Subject: [PATCH] Fix the order of the map (#268)

* Fix the order of the map

* Unify the sorting algorithm

* Put the sorting algorithm into an internal package

* Rename to represent the content of the file

* Remove the new sort library and tweak the old implementation
---
 describe.go               | 23 ++--------------------
 describe_test.go          | 19 +++++++++++++++++++
 field/ordered_map.go      |  5 ++---
 field/ordered_map_test.go | 40 +++++++++++++++++++++++++++++++++++++++
 sort/strings.go           |  6 +++---
 5 files changed, 66 insertions(+), 27 deletions(-)
 create mode 100644 field/ordered_map_test.go

diff --git a/describe.go b/describe.go
index 428b9286..e4826374 100644
--- a/describe.go
+++ b/describe.go
@@ -4,12 +4,11 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
-	"sort"
-	"strconv"
 	"strings"
 	"text/tabwriter"
 
 	"github.com/moov-io/iso8583/field"
+	moovsort "github.com/moov-io/iso8583/sort"
 )
 
 var defaultSpecName = "ISO 8583"
@@ -158,25 +157,7 @@ func sortFieldIDs(fields map[string]field.Field) []string {
 		keys = append(keys, k)
 	}
 
-	sort.Slice(keys, func(i, j int) bool {
-		// check if both keys are numeric
-		ni, ei := strconv.Atoi(keys[i])
-		nj, ej := strconv.Atoi(keys[j])
-		if ei == nil && ej == nil {
-			// if both keys are numeric, compare as integers
-			return ni < nj
-		}
-		if ei == nil {
-			// if only i is numeric, it goes first
-			return true
-		}
-		if ej == nil {
-			// if only j is numeric, it goes first
-			return false
-		}
-		// if neither key is numeric, compare as strings
-		return keys[i] < keys[j]
-	})
+	moovsort.StringsByInt(keys)
 
 	return keys
 }
diff --git a/describe_test.go b/describe_test.go
index baa060c9..bd8314e5 100644
--- a/describe_test.go
+++ b/describe_test.go
@@ -2,6 +2,7 @@ package iso8583
 
 import (
 	"bytes"
+	"reflect"
 	"testing"
 
 	"github.com/moov-io/iso8583/encoding"
@@ -133,3 +134,21 @@ func Test_splitAndAnnotate(t *testing.T) {
 		})
 	}
 }
+
+func TestSortFieldIDs(t *testing.T) {
+	var fields = map[string]field.Field{
+		"z":   field.NewString(nil),
+		"0":   field.NewString(nil),
+		"1":   field.NewString(nil),
+		"107": field.NewString(nil),
+		"a":   field.NewString(nil),
+		"17":  field.NewString(nil),
+		"2":   field.NewString(nil),
+		"4":   field.NewString(nil),
+	}
+	order := sortFieldIDs(fields)
+	expected := []string{"0", "1", "2", "4", "17", "107", "a", "z"}
+	if !reflect.DeepEqual(order, expected) {
+		t.Errorf("Marshalled value should be \n\t%v\ninstead of \n\t%v", expected, order)
+	}
+}
diff --git a/field/ordered_map.go b/field/ordered_map.go
index 197494cf..6be230df 100644
--- a/field/ordered_map.go
+++ b/field/ordered_map.go
@@ -4,8 +4,8 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"sort"
 
+	moovsort "github.com/moov-io/iso8583/sort"
 	"github.com/moov-io/iso8583/utils"
 )
 
@@ -17,8 +17,7 @@ func (om OrderedMap) MarshalJSON() ([]byte, error) {
 	for k := range om {
 		keys = append(keys, k)
 	}
-
-	sort.Strings(keys)
+	moovsort.StringsByInt(keys)
 
 	buf := &bytes.Buffer{}
 	buf.Write([]byte{'{'})
diff --git a/field/ordered_map_test.go b/field/ordered_map_test.go
new file mode 100644
index 00000000..f7fcfd75
--- /dev/null
+++ b/field/ordered_map_test.go
@@ -0,0 +1,40 @@
+package field
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+var (
+	testOrderedMap = OrderedMap{
+		"z":   &String{value: "555"},
+		"0":   &String{value: "0100"},
+		"1":   &String{value: "D00080000000000000000000002000000000000000000000"},
+		"107": &String{value: "102"},
+		"a":   &String{value: "555"},
+		"17":  &String{value: "101"},
+		"2":   &String{value: "4242424242424242"},
+		"4":   &String{value: "100"},
+	}
+)
+
+func TestOrderedMap_MarshalJSON(t *testing.T) {
+	expected := `{"0":"0100","1":"D00080000000000000000000002000000000000000000000","2":"4242424242424242","4":"100","17":"101","107":"102","a":"555","z":"555"}`
+	data, err := json.Marshal(testOrderedMap)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(data) != expected {
+		t.Errorf("Marshalled value should be \n\t%s\ninstead of \n\t%s", expected, string(data))
+	}
+}
+
+func BenchmarkOrderedMap_MarshalJSON(b *testing.B) {
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		_, err := json.Marshal(testOrderedMap)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
diff --git a/sort/strings.go b/sort/strings.go
index 4f262d97..7efa44b2 100644
--- a/sort/strings.go
+++ b/sort/strings.go
@@ -9,7 +9,7 @@ import (
 	"github.com/moov-io/iso8583/encoding"
 )
 
-// Strings is a function type used to sort a slice of strings in increasing
+// StringSlice is a function type used to sort a slice of strings in increasing
 // order. Any errors which arise from sorting the slice will raise a panic.
 type StringSlice func(x []string)
 
@@ -23,11 +23,11 @@ func StringsByInt(x []string) {
 	sort.Slice(x, func(i, j int) bool {
 		valI, err := strconv.Atoi(x[i])
 		if err != nil {
-			panic("failed to sort strings by int: failed to convert string to int")
+			return x[i] < x[j]
 		}
 		valJ, err := strconv.Atoi(x[j])
 		if err != nil {
-			panic("failed to sort strings by int: failed to convert string to int")
+			return x[i] < x[j]
 		}
 		return valI < valJ
 	})