Skip to content

change AmqpVersion to struct #927

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 19, 2020
Merged

change AmqpVersion to struct #927

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
21 changes: 16 additions & 5 deletions projects/RabbitMQ.Client/client/impl/AmqpVersion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
// Copyright (c) 2007-2020 VMware, Inc. All rights reserved.
//---------------------------------------------------------------------------

using System;

namespace RabbitMQ.Client.Framing.Impl
{
/// <summary>Represents a version of the AMQP specification.</summary>
Expand All @@ -45,7 +47,7 @@ namespace RabbitMQ.Client.Framing.Impl
/// special-cases 8-0, rewriting it at construction time to be 0-8 instead.
/// </para>
/// </remarks>
class AmqpVersion
internal readonly struct AmqpVersion : IEquatable<AmqpVersion>
{
/// <summary>
/// Construct an <see cref="AmqpVersion"/> from major and minor version numbers.
Expand All @@ -70,27 +72,36 @@ public AmqpVersion(int major, int minor)
/// <summary>
/// The AMQP specification major version number.
/// </summary>
public int Major { get; private set; }
public int Major { get; }

/// <summary>
/// The AMQP specification minor version number.
/// </summary>
public int Minor { get; private set; }
public int Minor { get; }

/// <summary>
/// Implement value-equality comparison.
/// </summary>
public override bool Equals(object other)
{
return (other is AmqpVersion version) && (version.Major == Major) && (version.Minor == Minor);
return other is AmqpVersion version && Equals(version);
}

public bool Equals(AmqpVersion other) => Major == other.Major && Minor == other.Minor;

public static bool operator ==(AmqpVersion left, AmqpVersion right) => left.Equals(right);

public static bool operator !=(AmqpVersion left, AmqpVersion right) => !left.Equals(right);

/// <summary>
/// Implement hashing as for value-equality.
/// </summary>
public override int GetHashCode()
{
return 31*Major.GetHashCode() + Minor.GetHashCode();
unchecked
{
return (Major * 397) ^ Minor;
}
Comment on lines +101 to +104
Copy link
Contributor

@sungam3r sungam3r Aug 18, 2020

Choose a reason for hiding this comment

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

Suggested change
unchecked
{
return (Major * 397) ^ Minor;
}
return (Major, Minor).GetHashCode();

Copy link
Contributor

@michaelklishin michaelklishin Aug 19, 2020

Choose a reason for hiding this comment

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

That seems to require an extra dependency on System.Tuple_2, though.

Copy link
Contributor

@michaelklishin michaelklishin Aug 19, 2020

Choose a reason for hiding this comment

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

Thank you for the suggestion, it is definitely a nicer option but I'm not sure if it's worth an extra dependency (if it does require one, I'm unable to compile this on .NET Core 3.1 on MacOS as is).

Copy link
Contributor

@sungam3r sungam3r Aug 19, 2020

Choose a reason for hiding this comment

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

ok

Copy link
Contributor Author

@bollhals bollhals Aug 19, 2020

Choose a reason for hiding this comment

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

Due to .NET 4.6.1 this syntax isn't yet possible to use without additional dependencies. (Similar to that we can't use anonymous (int, int) return values)

}

/// <summary>
Expand Down
8 changes: 2 additions & 6 deletions projects/RabbitMQ.Client/client/impl/Connection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1060,16 +1060,12 @@ void StartAndTune()

ServerProperties = connectionStart.m_serverProperties;

var serverVersion = new AmqpVersion(connectionStart.m_versionMajor,
connectionStart.m_versionMinor);
var serverVersion = new AmqpVersion(connectionStart.m_versionMajor, connectionStart.m_versionMinor);
if (!serverVersion.Equals(Protocol.Version))
{
TerminateMainloop();
FinishClose();
throw new ProtocolVersionMismatchException(Protocol.MajorVersion,
Protocol.MinorVersion,
serverVersion.Major,
serverVersion.Minor);
throw new ProtocolVersionMismatchException(Protocol.MajorVersion, Protocol.MinorVersion, serverVersion.Major, serverVersion.Minor);
}

ClientProperties = new Dictionary<string, object>(_factory.ClientProperties)
Expand Down