Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use ordinal string operations #766

Merged
merged 5 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions QuickFIXn.sln
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Examples.TradeClient", "Exa
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{FD384F84-3F83-4BF9-9238-82C70B74C2EA}"
ProjectSection(SolutionItems) = preProject
.editorconfig = .editorconfig
appveyor.yml = appveyor.yml
CONTRIBUTING.md = CONTRIBUTING.md
LICENSE = LICENSE
Expand Down
4 changes: 4 additions & 0 deletions QuickFIXn/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[*.cs]

# CA1310: Specify StringComparison for correctness
dotnet_diagnostic.CA1310.severity = error
2 changes: 1 addition & 1 deletion QuickFIXn/DefaultMessageFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ private static ICollection<Assembly> GetAppDomainAssemblies()
var assemblies = AppDomain
.CurrentDomain
.GetAssemblies()
.Where(assembly => !assembly.IsDynamic && assembly.GetName().Name.StartsWith("QuickFix"))
.Where(assembly => !assembly.IsDynamic && assembly.GetName().Name.StartsWith("QuickFix", StringComparison.Ordinal))
.ToList();
return assemblies;
}
Expand Down
7 changes: 3 additions & 4 deletions QuickFIXn/Fields/Converters/DateTimeConverter.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Collections.Generic;
using System.Text;
using System.Globalization;

namespace QuickFix.Fields.Converters
Expand Down Expand Up @@ -61,16 +60,16 @@ private static System.DateTime ConvertFromNanoString(string str, string[] format
System.DateTimeKind kind;
int offset = 0;

if (dec.EndsWith("Z"))
if (dec.EndsWith('Z'))
{
// UTC
dec = dec.Substring(0, dec.Length - 1);
kind = System.DateTimeKind.Utc;
}
else if (dec.Contains("+") || dec.Contains("-"))
else if (dec.Contains('+') || dec.Contains('-'))
{
// GMT offset
int n = dec.Contains("+") ? dec.IndexOf('+') : dec.IndexOf('-');
int n = dec.Contains('+') ? dec.IndexOf('+') : dec.IndexOf('-');
kind = System.DateTimeKind.Unspecified;
offset = int.Parse(dec.Substring(n));
dec = dec.Substring(0, n);
Expand Down
5 changes: 2 additions & 3 deletions QuickFIXn/Message/Message.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Text.RegularExpressions;
using System.Text.Json;
using System.Collections.Generic;
using QuickFix.Util;

namespace QuickFix
{
Expand Down Expand Up @@ -121,7 +120,7 @@ public Message(Message src)

public static bool IsAdminMsgType(string msgType)
{
return msgType.Length == 1 && "0A12345n".IndexOf(msgType) != -1;
return msgType.Length == 1 && "0A12345n".Contains(msgType[0]);
}

/// <summary>
Expand Down Expand Up @@ -682,7 +681,7 @@ public void ReverseRoute(Header header)
this.Header.RemoveField(Tags.OnBehalfOfLocationID);
this.Header.RemoveField(Tags.DeliverToLocationID);

if (StringUtil.InvariantCompareTo(beginString, FixValues.BeginString.FIX41) >= 0)
if (string.CompareOrdinal(beginString, "FIX.4.1") >= 0)
Copy link

@maxnis maxnis Mar 27, 2023

Choose a reason for hiding this comment

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

As recommended by Microsoft - it's better to use Equals method to test equality, isn't it ?

if (beginString.Equals("FIX.4.1", StringComparison.InvariantCulture))

also, magic string "FIX.4.1" - can it be converted to a const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, but here we are not checking for equality (of FIX version 4.1), rather that we are at version 4.1 or higher (from the >=).

{
if (header.IsSetField(Tags.OnBehalfOfLocationID))
{
Expand Down
21 changes: 10 additions & 11 deletions QuickFIXn/Session.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Threading;
using QuickFix.Fields;
using QuickFix.Fields.Converters;
using QuickFix.Util;

namespace QuickFix
{
Expand Down Expand Up @@ -651,7 +650,7 @@ internal void Next(MessageBuilder msgBuilder)
catch (FieldNotFoundException e)
{
Log.OnEvent("Rejecting invalid message, field not found: " + e.Message);
if (StringUtil.InvariantCompareTo(SessionID.BeginString, FixValues.BeginString.FIX42) >= 0 && (message.IsApp()))
if ((string.CompareOrdinal(SessionID.BeginString, FixValues.BeginString.FIX42) >= 0) && (message.IsApp()))
{
GenerateBusinessMessageReject(message, Fields.BusinessRejectReason.CONDITIONALLY_REQUIRED_FIELD_MISSING, e.Field);
}
Expand Down Expand Up @@ -1045,7 +1044,7 @@ private void initializeResendFields(Message message)

protected bool ShouldSendReset()
{
return (StringUtil.InvariantCompareTo(this.SessionID.BeginString, FixValues.BeginString.FIX41) >= 0)
return (string.CompareOrdinal(SessionID.BeginString, FixValues.BeginString.FIX41) >= 0)
&& (this.ResetOnLogon || this.ResetOnLogout || this.ResetOnDisconnect)
&& (_state.NextSenderMsgSeqNum == 1)
&& (_state.NextTargetMsgSeqNum == 1);
Expand Down Expand Up @@ -1149,7 +1148,7 @@ protected void GenerateBusinessMessageReject(Message message, int err, int field
SeqNumType msgSeqNum = message.Header.GetULong(Tags.MsgSeqNum);
string reason = FixValues.BusinessRejectReason.RejText[err];
Message reject;
if (StringUtil.InvariantCompareTo(this.SessionID.BeginString, FixValues.BeginString.FIX42) >= 0)
if (string.CompareOrdinal(SessionID.BeginString, FixValues.BeginString.FIX42) >= 0)
{
reject = _msgFactory.Create(this.SessionID.BeginString, MsgType.BUSINESS_MESSAGE_REJECT);
reject.SetField(new RefMsgType(msgType));
Expand Down Expand Up @@ -1203,9 +1202,9 @@ protected void GenerateResendRequest(string beginString, SeqNumType msgSeqNum)
}
else
{
if (StringUtil.InvariantCompareTo(beginString, FixValues.BeginString.FIX42) >= 0)
if (string.CompareOrdinal(beginString, FixValues.BeginString.FIX42) >= 0)
endRangeSeqNum = 0;
else if (StringUtil.InvariantCompareTo(beginString, FixValues.BeginString.FIX41) <= 0)
else if (string.CompareOrdinal(beginString, FixValues.BeginString.FIX41) <= 0)
endRangeSeqNum = 999999;
endChunkSeqNum = endRangeSeqNum;
}
Expand Down Expand Up @@ -1392,11 +1391,11 @@ public void GenerateReject(Message message, FixValues.SessionRejectReason reason
{ }
}

if (StringUtil.InvariantCompareTo(beginString, FixValues.BeginString.FIX42) >= 0)
if (string.CompareOrdinal(beginString, FixValues.BeginString.FIX42) >= 0)
{
if (msgType.Length > 0)
reject.SetField(new Fields.RefMsgType(msgType));
if ((FixValues.BeginString.FIX42.Equals(beginString) && reason.Value <= FixValues.SessionRejectReason.INVALID_MSGTYPE.Value) || StringUtil.InvariantCompareTo(beginString, FixValues.BeginString.FIX42) > 0)
if ((FixValues.BeginString.FIX42.Equals(beginString) && reason.Value <= FixValues.SessionRejectReason.INVALID_MSGTYPE.Value) || (string.CompareOrdinal(beginString, FixValues.BeginString.FIX42) > 0))
{
reject.SetField(new Fields.SessionRejectReason(reason.Value));
}
Expand All @@ -1412,7 +1411,7 @@ public void GenerateReject(Message message, FixValues.SessionRejectReason reason
{
if (FixValues.SessionRejectReason.INVALID_MSGTYPE.Equals(reason))
{
if (StringUtil.InvariantCompareTo(this.SessionID.BeginString, FixValues.BeginString.FIX43) >= 0)
if (string.CompareOrdinal(SessionID.BeginString, FixValues.BeginString.FIX43) >= 0)
PopulateRejectReason(reject, reason.Description);
else
PopulateSessionRejectReason(reject, field, reason.Description, false);
Expand All @@ -1436,7 +1435,7 @@ public void GenerateReject(Message message, FixValues.SessionRejectReason reason

protected void PopulateSessionRejectReason(Message reject, int field, string text, bool includeFieldInfo)
{
if (StringUtil.InvariantCompareTo(this.SessionID.BeginString, FixValues.BeginString.FIX42) >= 0)
if (string.CompareOrdinal(SessionID.BeginString, FixValues.BeginString.FIX42) >= 0)
{
reject.SetField(new Fields.RefTagID(field));
reject.SetField(new Fields.Text(text));
Expand Down Expand Up @@ -1494,7 +1493,7 @@ protected void InitializeHeader(Message m)

private bool IsFix42OrAbove() {
return this.SessionID.BeginString == FixValues.BeginString.FIXT11
|| StringUtil.InvariantCompareTo(this.SessionID.BeginString, FixValues.BeginString.FIX42) >= 0;
|| string.CompareOrdinal(SessionID.BeginString, FixValues.BeginString.FIX42) >= 0;
}

protected void InsertSendingTime(FieldMap header)
Expand Down
2 changes: 1 addition & 1 deletion QuickFIXn/SessionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ protected void ProcessFixTDataDictionaries(SessionID sessionID, Dictionary setti
}
else
{
int offset = setting.Key.IndexOf(".");
int offset = setting.Key.IndexOf('.');
if (offset == -1)
throw new System.ArgumentException(string.Format("Malformed {0} : {1}", SessionSettings.APP_DATA_DICTIONARY, setting.Key));

Expand Down
2 changes: 1 addition & 1 deletion QuickFIXn/SessionID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public SessionID(string beginString, string senderCompID, string senderSubID, st
targetSubID_ = targetSubID;
targetLocationID_ = targetLocationID;
sessionQualifier_ = sessionQualifier;
isFIXT_ = beginString_.StartsWith("FIXT");
isFIXT_ = beginString_.StartsWith("FIXT", StringComparison.Ordinal);

id_ = beginString_
+ ":"
Expand Down
2 changes: 1 addition & 1 deletion QuickFIXn/Transport/StreamFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private static Socket CreateTunnelThruProxy(string destIP, int destPort)
int msg = socketThruProxy.Receive(buffer12, 500, 0);
string data;
data = Encoding.ASCII.GetString(buffer12);
int index = data.IndexOf("200");
int index = data.IndexOf("200", StringComparison.Ordinal);

if (index < 0)
throw new ApplicationException(
Expand Down
5 changes: 1 addition & 4 deletions QuickFIXn/Util/StringUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ public static string FixSlashes(string s) {
? s.Replace('/', '\\')
: s.Replace('\\', '/');
}

public static int InvariantCompareTo(string strA, string strB) {
return string.Compare(strA, strB, StringComparison.InvariantCulture);
}
}
}

3 changes: 2 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ What's New

### NEXT RELEASE

**Breaking change**
**Breaking changes**
* #768 - span-ify parser (Rob-Hague) - makes a change to QuickFix.Parser interface, which isn't likely to affect users
* #820 - cleanup/nullable-ize files (gbirchmeier) - changed some Session Generate* functions to return void instead of null.
Very low likelihood that any user code will be affected.
Expand All @@ -25,6 +25,7 @@ What's New
* #813 - fix incorrect logging of acceptor heartbeat (gbirchmeier)
* #815 - update broken/neglected example apps & docs (gbirchmeier)
* #764 - fix positive UTC offset parsing in DateTimeConverter (Rob-Hague)
* #766 - use ordinal string operations (Rob-Hague)

### v1.11.2:
* same as v1.11.1, but I fixed the readme in the pushed nuget packages
Expand Down
2 changes: 1 addition & 1 deletion UnitTests/DataDictionaryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ public void ParseThroughComments()
XmlNode MakeNode(string xmlString)
{
XmlDocument doc = new XmlDocument();
if (xmlString.StartsWith("<"))
if (xmlString.StartsWith("<", StringComparison.Ordinal))
{
doc.LoadXml(xmlString);
return doc.DocumentElement;
Expand Down
2 changes: 1 addition & 1 deletion UnitTests/ThreadedSocketReactorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void TestStartOnBusyPort()
Assert.IsNotNull(stdOutResult);

Assert.AreEqual(typeof(SocketException), exceptionResult.GetType());
Assert.IsTrue(stdOutResult.StartsWith("Error starting listener: "));
Assert.IsTrue(stdOutResult.StartsWith("Error starting listener: ", StringComparison.Ordinal));
}

[TearDown]
Expand Down
3 changes: 0 additions & 3 deletions UnitTests/Util/UtcDateTimeSerializerTests.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using NUnit.Framework;
using QuickFix.Util;

Expand Down
Loading