Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optimize quickfixType Function for Performance & Readability #691

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Optimize quickfixType Function for Performance & Readability #691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 32 additions & 81 deletions cmd/generate-fix/internal/template_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,91 +199,42 @@ func quickfixValueType(quickfixType string) (goType string, err error) {
}

func quickfixType(field *datadictionary.FieldType) (quickfixType string, err error) {
switch field.Type {
case "MULTIPLESTRINGVALUE", "MULTIPLEVALUESTRING":
fallthrough
case "MULTIPLECHARVALUE":
fallthrough
case "CHAR":
fallthrough
case "CURRENCY":
fallthrough
case "DATA":
fallthrough
case "MONTHYEAR":
fallthrough
case "LOCALMKTTIME", "LOCALMKTDATE":
fallthrough
case "TIME":
fallthrough
case "DATE":
fallthrough
case "EXCHANGE":
fallthrough
case "LANGUAGE":
fallthrough
case "XMLDATA":
fallthrough
case "COUNTRY":
fallthrough
case "UTCTIMEONLY":
fallthrough
case "UTCDATE":
fallthrough
case "UTCDATEONLY":
fallthrough
case "TZTIMEONLY":
fallthrough
case "TZTIMESTAMP":
fallthrough
case "XID", "XIDREF":
fallthrough
case "STRING":
quickfixType = "FIXString"

case "BOOLEAN":
quickfixType = "FIXBoolean"

case "LENGTH":
fallthrough
case "DAYOFMONTH":
fallthrough
case "NUMINGROUP":
fallthrough
case "SEQNUM":
fallthrough
case "TAGNUM":
fallthrough
case "INT":
quickfixType = "FIXInt"

case "UTCTIMESTAMP":
quickfixType = "FIXUTCTimestamp"

case "QTY":
fallthrough
case "QUANTITY":
fallthrough
case "AMT":
fallthrough
case "PRICE":
fallthrough
case "PRICEOFFSET":
fallthrough
case "PERCENTAGE":
fallthrough
case "FLOAT":
// Define mappings of FIX field types to QuickFIX types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I do like the change here because it’s code generation and readability is more important than performance in this regard

stringTypes := map[string]bool{
"MULTIPLESTRINGVALUE": true, "MULTIPLEVALUESTRING": true, "MULTIPLECHARVALUE": true,
"CHAR": true, "CURRENCY": true, "DATA": true, "MONTHYEAR": true,
"LOCALMKTTIME": true, "LOCALMKTDATE": true, "TIME": true, "DATE": true,
"EXCHANGE": true, "LANGUAGE": true, "XMLDATA": true, "COUNTRY": true,
"UTCTIMEONLY": true, "UTCDATE": true, "UTCDATEONLY": true, "TZTIMEONLY": true,
"TZTIMESTAMP": true, "XID": true, "XIDREF": true, "STRING": true,
}
intTypes := map[string]bool{
"LENGTH": true, "DAYOFMONTH": true, "NUMINGROUP": true,
"SEQNUM": true, "TAGNUM": true, "INT": true,
}

floatTypes := map[string]bool{
"QTY": true, "QUANTITY": true, "AMT": true,
"PRICE": true, "PRICEOFFSET": true, "PERCENTAGE": true, "FLOAT": true,
}

switch {
case stringTypes[field.Type]:
return "FIXString", nil
case intTypes[field.Type]:
return "FIXInt", nil
case field.Type == "BOOLEAN":
return "FIXBoolean", nil
case field.Type == "UTCTIMESTAMP":
return "FIXUTCTimestamp", nil
case floatTypes[field.Type]:
if *useFloat {
quickfixType = "FIXFloat"
} else {
quickfixType = "FIXDecimal"
return "FIXFloat", nil
}

return "FIXDecimal", nil
default:
err = fmt.Errorf("Unknown type '%v' for tag '%v'\n", field.Type, field.Tag())
return "", fmt.Errorf("Unknown type '%v' for tag '%v'\n", field.Type, field.Tag())
}

return
}

func requiredFields(m *datadictionary.MessageDef) (required []*datadictionary.FieldDef) {
Expand Down
97 changes: 33 additions & 64 deletions validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,14 @@ func validateField(d *datadictionary.DataDictionary,
_ datadictionary.TagSet,
field TagValue,
) MessageRejectError {
// If the field has no value, return an error
if len(field.value) == 0 {
return TagSpecifiedWithoutAValue(field.tag)
}

fieldType, isMessageField := getFieldType(d, int(field.tag))

// If the field is neither a message field nor explicitly allowed, return an error
if !isMessageField && !checkFieldNotDefined(settings, field.tag) {
return InvalidTagNumber(field.tag)
}
Expand All @@ -377,80 +380,46 @@ func validateField(d *datadictionary.DataDictionary,
return nil
}

// Validate against allowed enumerations
allowedValues := d.FieldTypeByTag[int(field.tag)].Enums
if len(allowedValues) != 0 {
if _, validValue := allowedValues[string(field.value)]; !validValue {
return ValueIsIncorrect(field.tag)
}
}
// Define field type mappings for easy lookup
fieldTypeMapping := map[string]FieldValue{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This instantiation performs a lot of unnecessary allocations which will negatively impact performance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the addition of comments on this path to help the developer, but on a hot path (versus the code generation path you changed otherwise) a switch statement will perform better than a map lookup. A blurb on why that’s the case: https://stackoverflow.com/questions/46789259/map-vs-switch-performance-in-go

"MULTIPLESTRINGVALUE": new(FIXString), "MULTIPLEVALUESTRING": new(FIXString),
"MULTIPLECHARVALUE": new(FIXString), "CHAR": new(FIXString),
"CURRENCY": new(FIXString), "DATA": new(FIXString),
"MONTHYEAR": new(FIXString), "LOCALMKTDATE": new(FIXString),
"DATE": new(FIXString), "EXCHANGE": new(FIXString),
"LANGUAGE": new(FIXString), "XMLDATA": new(FIXString),
"COUNTRY": new(FIXString), "UTCTIMEONLY": new(FIXString),
"UTCDATEONLY": new(FIXString), "UTCDATE": new(FIXString),
"TZTIMEONLY": new(FIXString), "TZTIMESTAMP": new(FIXString),
"STRING": new(FIXString),

"BOOLEAN": new(FIXBoolean),

var prototype FieldValue
switch fieldType.Type {
case "MULTIPLESTRINGVALUE", "MULTIPLEVALUESTRING":
fallthrough
case "MULTIPLECHARVALUE":
fallthrough
case "CHAR":
fallthrough
case "CURRENCY":
fallthrough
case "DATA":
fallthrough
case "MONTHYEAR":
fallthrough
case "LOCALMKTDATE", "DATE":
fallthrough
case "EXCHANGE":
fallthrough
case "LANGUAGE":
fallthrough
case "XMLDATA":
fallthrough
case "COUNTRY":
fallthrough
case "UTCTIMEONLY":
fallthrough
case "UTCDATEONLY", "UTCDATE":
fallthrough
case "TZTIMEONLY":
fallthrough
case "TZTIMESTAMP":
fallthrough
case "STRING":
prototype = new(FIXString)

case "BOOLEAN":
prototype = new(FIXBoolean)

case "LENGTH":
fallthrough
case "DAYOFMONTH":
fallthrough
case "NUMINGROUP":
fallthrough
case "SEQNUM":
fallthrough
case "INT":
prototype = new(FIXInt)

case "UTCTIMESTAMP", "TIME":
prototype = new(FIXUTCTimestamp)

case "QTY", "QUANTITY":
fallthrough
case "AMT":
fallthrough
case "PRICE":
fallthrough
case "PRICEOFFSET":
fallthrough
case "PERCENTAGE":
fallthrough
case "FLOAT":
prototype = new(FIXFloat)
"LENGTH": new(FIXInt), "DAYOFMONTH": new(FIXInt),
"NUMINGROUP": new(FIXInt), "SEQNUM": new(FIXInt),
"INT": new(FIXInt),

"UTCTIMESTAMP": new(FIXUTCTimestamp), "TIME": new(FIXUTCTimestamp),

"QTY": new(FIXFloat), "QUANTITY": new(FIXFloat),
"AMT": new(FIXFloat), "PRICE": new(FIXFloat),
"PRICEOFFSET": new(FIXFloat), "PERCENTAGE": new(FIXFloat),
"FLOAT": new(FIXFloat),
}
// Get the appropriate prototype for field validation
prototype, exists := fieldTypeMapping[fieldType.Type]
if !exists {
return nil // If the field type is not recognized, no validation needed
}

// Validate the field's value format
if err := prototype.Read(field.value); err != nil {
return IncorrectDataFormatForValue(field.tag)
}
Expand Down
Loading