Skip to content

Refactor module specification logic #7126

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
iSazonov merged 11 commits into from
Jul 3, 2018
Merged

Refactor module specification logic #7126

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
17c87b1
Use nameof in module specification argument errors
Jun 21, 2018
c8fe6b2
Factor out field ToString() call
Jun 21, 2018
6de7b61
Use StringBuilder for ToString() implementation
Jun 21, 2018
4d2d568
Simplify equality comparison logic
Jun 21, 2018
03d1c3a
[Feature] Simplify hashcode logic
Jun 21, 2018
77d2646
Use better hashing algorithm
Jun 21, 2018
89370a5
Add null check to GetHashCode() method
Jun 21, 2018
7ef9701
Improve unchecked usage
Jun 21, 2018
8d34597
Change multiplication prime to use 31
Jun 21, 2018
74f1b34
Unroll loop in GetHashCode to prevent allocation
rjmholt Jun 22, 2018
cd065f8
Use corefx's HashCode.Combine() method
Jun 26, 2018
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
164 changes: 60 additions & 104 deletions src/System.Management.Automation/engine/Modules/ModuleSpecification.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public ModuleSpecification(string moduleName)
{
if (string.IsNullOrEmpty(moduleName))
{
throw new ArgumentNullException("moduleName");
throw new ArgumentNullException(nameof(moduleName));
}
this.Name = moduleName;
// Alias name of miniumVersion
Expand All @@ -67,7 +67,7 @@ public ModuleSpecification(Hashtable moduleSpecification)
{
if (moduleSpecification == null)
{
throw new ArgumentNullException("moduleSpecification");
throw new ArgumentNullException(nameof(moduleSpecification));
}

var exception = ModuleSpecificationInitHelper(this, moduleSpecification);
Expand All @@ -91,31 +91,35 @@ internal static Exception ModuleSpecificationInitHelper(ModuleSpecification modu
{
foreach (DictionaryEntry entry in hashtable)
{
if (entry.Key.ToString().Equals("ModuleName", StringComparison.OrdinalIgnoreCase))
string field = entry.Key.ToString();

if (field.Equals("ModuleName", StringComparison.OrdinalIgnoreCase))
{
moduleSpecification.Name = LanguagePrimitives.ConvertTo<string>(entry.Value);
}
else if (entry.Key.ToString().Equals("ModuleVersion", StringComparison.OrdinalIgnoreCase))
else if (field.Equals("ModuleVersion", StringComparison.OrdinalIgnoreCase))
{
moduleSpecification.Version = LanguagePrimitives.ConvertTo<Version>(entry.Value);
}
else if (entry.Key.ToString().Equals("RequiredVersion", StringComparison.OrdinalIgnoreCase))
else if (field.Equals("RequiredVersion", StringComparison.OrdinalIgnoreCase))
{
moduleSpecification.RequiredVersion = LanguagePrimitives.ConvertTo<Version>(entry.Value);
}
else if (entry.Key.ToString().Equals("MaximumVersion", StringComparison.OrdinalIgnoreCase))
else if (field.Equals("MaximumVersion", StringComparison.OrdinalIgnoreCase))
{
moduleSpecification.MaximumVersion = LanguagePrimitives.ConvertTo<String>(entry.Value);
ModuleCmdletBase.GetMaximumVersion(moduleSpecification.MaximumVersion);
}
else if (entry.Key.ToString().Equals("GUID", StringComparison.OrdinalIgnoreCase))
else if (field.Equals("GUID", StringComparison.OrdinalIgnoreCase))
{
moduleSpecification.Guid = LanguagePrimitives.ConvertTo<Guid?>(entry.Value);
}
else
{
if (badKeys.Length > 0)
{
badKeys.Append(", ");
}
badKeys.Append("'");
badKeys.Append(entry.Key.ToString());
badKeys.Append("'");
Expand Down Expand Up @@ -166,7 +170,7 @@ internal ModuleSpecification(PSModuleInfo moduleInfo)
{
if (moduleInfo == null)
{
throw new ArgumentNullException("moduleInfo");
throw new ArgumentNullException(nameof(moduleInfo));
}

this.Name = moduleInfo.Name;
Expand All @@ -182,36 +186,39 @@ internal ModuleSpecification(PSModuleInfo moduleInfo)
/// <returns></returns>
public override string ToString()
{
string moduleSpecString = string.Empty;
if (Guid == null && Version == null && RequiredVersion == null && MaximumVersion == null)
{
moduleSpecString = Name;
return Name;
}

var moduleSpecBuilder = new StringBuilder();

moduleSpecBuilder.Append("@{ ModuleName = '").Append(Name).Append("'");

if (Guid != null)
{
moduleSpecBuilder.Append("; Guid = '{").Append(Guid).Append("}' ");
}

if (RequiredVersion != null)
{
moduleSpecBuilder.Append("; RequiredVersion = '").Append(RequiredVersion).Append("'");
}
else
{
moduleSpecString = "@{ ModuleName = '" + Name + "'";
if (Guid != null)
{
moduleSpecString += "; Guid = '{" + Guid + "}' ";
}
if (RequiredVersion != null)
if (Version != null)
{
moduleSpecString += "; RequiredVersion = '" + RequiredVersion + "'";
moduleSpecBuilder.Append("; ModuleVersion = '").Append(Version).Append("'");
}
else
if (MaximumVersion != null)
{
if (Version != null)
{
moduleSpecString += "; ModuleVersion = '" + Version + "'";
}
if (MaximumVersion != null)
{
moduleSpecString += "; MaximumVersion = '" + MaximumVersion + "'";
}
moduleSpecBuilder.Append("; MaximumVersion = '").Append(MaximumVersion).Append("'");
}
moduleSpecString += " }";
}
return moduleSpecString;

moduleSpecBuilder.Append(" }");

return moduleSpecBuilder.ToString();
}

/// <summary>
Expand Down Expand Up @@ -266,96 +273,45 @@ public static bool TryParse(string input, out ModuleSpecification result)
public Version RequiredVersion { get; internal set; }
}

/// <summary>
/// Compares two ModuleSpecification objects for equality.
/// </summary>
internal class ModuleSpecificationComparer : IEqualityComparer<ModuleSpecification>
{
/// <summary>
/// Check if two module specifications are property-wise equal.
/// </summary>
/// <param name="x"></param>
/// <param name="y"></param>
/// <returns>True if the specifications are equal, false otherwise.</returns>
public bool Equals(ModuleSpecification x, ModuleSpecification y)
{
bool result = false;

if (x == null && y == null)
if (x == y)
{
result = true;
}
else if (x != null && y != null)
{
if (x.Name != null && y.Name != null)
{
result = x.Name.Equals(y.Name, StringComparison.OrdinalIgnoreCase);
}
else
{
result = true;
}
if (result)
{
if (x.Guid.HasValue && y.Guid.HasValue)
{
result = x.Guid.Equals(y.Guid);
}
}
if (result)
{
if (x.Version != null && y.Version != null)
{
result = x.Version.Equals(y.Version);
}
else if (x.Version != null || y.Version != null)
{
result = false;
}

if (x.MaximumVersion != null && y.MaximumVersion != null)
{
result = x.MaximumVersion.Equals(y.MaximumVersion);
}
else if (x.MaximumVersion != null || y.MaximumVersion != null)
{
result = false;
}

if (result && x.RequiredVersion != null && y.RequiredVersion != null)
{
result = x.RequiredVersion.Equals(y.RequiredVersion);
}
else if (result && (x.RequiredVersion != null || y.RequiredVersion != null))
{
result = false;
}
}
return true;
}

return result;
return x != null && y != null
&& String.Equals(x.Name, y.Name, StringComparison.OrdinalIgnoreCase)
&& Guid.Equals(x.Guid, y.Guid)
&& Version.Equals(x.RequiredVersion, y.RequiredVersion)
&& Version.Equals(x.Version, y.Version)
&& String.Equals(x.MaximumVersion, y.MaximumVersion);
}

/// <summary>
/// Get a property-based hashcode for a ModuleSpecification object.
/// </summary>
/// <param name="obj">The module specification for the object.</param>
/// <returns>A hashcode that is always the same for any module specification with the same properties.</returns>
public int GetHashCode(ModuleSpecification obj)
{
int result = 0;

if (obj != null)
if (obj == null)
{
if (obj.Name != null)
{
result = result ^ obj.Name.GetHashCode();
}
if (obj.Guid.HasValue)
{
result = result ^ obj.Guid.GetHashCode();
}
if (obj.Version != null)
{
result = result ^ obj.Version.GetHashCode();
}
if (obj.MaximumVersion != null)
{
result = result ^ obj.MaximumVersion.GetHashCode();
}
if (obj.RequiredVersion != null)
{
result = result ^ obj.RequiredVersion.GetHashCode();
}
return 0;
}

return result;
return HashCode.Combine(obj.Name, obj.Guid, obj.RequiredVersion, obj.Version, obj.MaximumVersion);
}
}

Expand Down