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 nameof instead of strongly typed string #1896

Merged
merged 4 commits into from Jul 18, 2019
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -87,7 +87,7 @@ public static FilterModel FromFullLine(string line)
}
else
{
throw new FormatException("Could not parse Height.");
throw new FormatException($"Could not parse {nameof(Height)}.");
}
}

@@ -93,7 +93,7 @@ internal void Add(int value, int bitCount)
{
if (bitCount < 0 || bitCount > 32)
{
throw new ArgumentOutOfRangeException(nameof(bitCount), "bitCount must be greater than or equal to 0");
throw new ArgumentOutOfRangeException(nameof(bitCount), $"{nameof(bitCount)} must be greater than or equal to 0");
}

int numBitsLeft = bitCount;
@@ -211,25 +211,25 @@ protected Option(string prototype, string description, int maxValueCount, bool h
if (MaxValueCount == 0 && OptionValueType != OptionValueType.None)
{
throw new ArgumentException(
"Cannot provide maxValueCount of 0 for OptionValueType.Required or " +
"OptionValueType.Optional.",
nameof(maxValueCount));
$"Cannot provide {nameof(maxValueCount)} of 0 for {nameof(OptionValueType)}.{nameof(OptionValueType.Required)}" +
$" or {nameof(OptionValueType)}.{nameof(OptionValueType.Optional)}.",
nameof(maxValueCount));
}

if (OptionValueType == OptionValueType.None && maxValueCount > 1)
{
throw new ArgumentException(
$"Cannot provide maxValueCount of {maxValueCount} for OptionValueType.None.",
nameof(maxValueCount));
$"Cannot provide {nameof(maxValueCount)} of {maxValueCount} for {nameof(OptionValueType)}.{nameof(OptionValueType.None)}.",
nameof(maxValueCount));
}

if (Array.IndexOf(Names, "<>") >= 0 &&
((Names.Length == 1 && OptionValueType != OptionValueType.None) ||
(Names.Length > 1 && MaxValueCount > 1)))
{
throw new ArgumentException(
"The default option handler '<>' cannot require values.",
nameof(prototype));
"The default option handler '<>' cannot require values.",
nameof(prototype));
}
}

@@ -278,10 +278,8 @@ protected static T Parse<T>(string value, OptionContext c)
catch (Exception e)
{
throw new OptionException(
string.Format(
c.OptionSet.MessageLocalizer("Could not convert string `{0}' to type {1} for option `{2}'."),
value, targetType.Name, c.OptionName),
c.OptionName, e);
string.Format(c.OptionSet.MessageLocalizer($"Could not convert string `{value}' to type {targetType.Name} for option `{c.OptionName}'.")),
c.OptionName, e);
}
return t;
}
@@ -300,7 +298,7 @@ private OptionValueType ParsePrototype()
string name = Names[i];
if (name.Length == 0)
{
throw new ArgumentException("Empty option names are not supported.", "prototype");
throw new ArgumentException("Empty option names are not supported.", nameof(Prototype).ToLower());

This comment has been minimized.

Copy link
@molnard

molnard Jul 17, 2019

Collaborator

Bacause ArgumentException second parameter is the exact name of param, IMO .ToLower() is unnecessary even if the original Exception worked like that. These texts are written to the user on the console so changing it won't cause any trouble.

This comment has been minimized.

Copy link
@yahiheb

yahiheb Jul 17, 2019

Author Collaborator

In other exception messages "prototype" all lower case was used, that is the only reason I used .ToLower(). I can remove it if you want.

This comment has been minimized.

Copy link
@molnard

molnard Jul 17, 2019

Collaborator

@nopara73 Should we stay with prototype with lower case in ArgumentException? The name of the property is Prototype so that would be the correct one but in the original code it is prototype.
These messages are written to the console to the user.

}

int end = name.IndexOfAny(NameTerminator);
@@ -318,7 +316,7 @@ private OptionValueType ParsePrototype()
{
throw new ArgumentException(
$"Conflicting option types: '{type}' vs. '{name[end]}'.",
"prototype");
nameof(Prototype).ToLower());
}

AddSeparators(name, end, seps);
@@ -335,7 +333,7 @@ private OptionValueType ParsePrototype()
{
throw new ArgumentException(
$"Cannot provide key/value separators for Options taking {MaxValueCount} value(s).",
"prototype");
nameof(Prototype).ToLower());
}
}
else
@@ -369,7 +367,7 @@ private static void AddSeparators(string name, int end, ICollection<string> seps
{
throw new ArgumentException(
$"Ill-formed name/value separator found in \"{name}\".",
"prototype");
nameof(Prototype).ToLower());
}

start = i + 1;
@@ -380,7 +378,7 @@ private static void AddSeparators(string name, int end, ICollection<string> seps
{
throw new ArgumentException(
$"Ill-formed name/value separator found in \"{name}\".",
"prototype");
nameof(Prototype).ToLower());
}

seps.Add(name.Substring(start, i - start));
@@ -400,7 +398,7 @@ private static void AddSeparators(string name, int end, ICollection<string> seps
{
throw new ArgumentException(
$"Ill-formed name/value separator found in \"{name}\".",
"prototype");
nameof(Prototype).ToLower());
}
}

@@ -196,7 +196,7 @@ protected override string GetKeyForItem(Option item)
{
if (item is null)
{
throw new ArgumentNullException("option");
throw new ArgumentNullException(nameof(Option).ToLower());
This conversation was marked as resolved by yahiheb

This comment has been minimized.

Copy link
@molnard

molnard Jul 17, 2019

Collaborator

Same as above. Remove .ToLower()

}

if (item.Names != null && item.Names.Length > 0)
@@ -205,7 +205,7 @@ protected override string GetKeyForItem(Option item)
}
// This should never happen, as it's invalid for Option to be
// constructed w/o any names.
throw new InvalidOperationException("Option has no names!");
throw new InvalidOperationException($"{nameof(Option)} has no names!");
}

[Obsolete("Use KeyedCollection.this[string]")]
@@ -294,13 +294,13 @@ public sealed class Category : Option
// (see Option.ParsePrototype(), and thus it'll prevent Category
// instances from being accidentally used as normal options.
public Category(string description)
: base("=:Category:= " + description, description)
: base($"=:{nameof(Category)}:= " + description, description)
{
}

protected override void OnParseComplete(OptionContext c)
{
throw new NotSupportedException("Category.OnParseComplete should not be invoked.");
throw new NotSupportedException($"{nameof(Category)}.{nameof(OnParseComplete)} should not be invoked.");
}
}

@@ -649,8 +649,8 @@ private void ParseValue(string option, OptionContext c)
else if (c.OptionValues.Count > c.Option.MaxValueCount)
{
throw new OptionException(MessageLocalizer(
$"Error: Found {c.OptionValues.Count} option values when expecting {c.Option.MaxValueCount}."),
c.OptionName);
$"Error: Found {c.OptionValues.Count} option values when expecting {c.Option.MaxValueCount}."),
c.OptionName);
}
}

@@ -692,7 +692,7 @@ private bool ParseBundledValue(string f, string n, OptionContext c)
}

throw new OptionException(string.Format(MessageLocalizer(
"Cannot use unregistered option '{0}' in bundle '{1}'."), rn, f + n), null);
$"Cannot use unregistered option '{rn}' in bundle '{f + n}'.")), null);
}
p = this[rn];
switch (p.OptionValueType)
@@ -711,7 +711,7 @@ private bool ParseBundledValue(string f, string n, OptionContext c)
return true;
}
default:
throw new InvalidOperationException("Unknown OptionValueType: " + p.OptionValueType);
throw new InvalidOperationException($"Unknown {nameof(OptionValueType)}: " + p.OptionValueType);
This conversation was marked as resolved by yahiheb

This comment has been minimized.

Copy link
@molnard

molnard Jul 17, 2019

Collaborator

Don't need to concatenate. Is this work?
$"Unknown {nameof(OptionValueType)}: {p.OptionValueType}"

}
}
return true;
@@ -960,7 +960,7 @@ private static string GetDescription(string description)
{
if ((i + 1) == description.Length || description[i + 1] != '}')
{
throw new InvalidOperationException("Invalid option description: " + description);
throw new InvalidOperationException($"Invalid option description: {description}");
}

++i;
@@ -1,4 +1,4 @@
//
//
// Options.cs
//
// Authors:
@@ -293,7 +293,7 @@ private void AssertValid(int index)
{
if (C.Option is null)
{
throw new InvalidOperationException("OptionContext.Option is null.");
throw new InvalidOperationException($"{nameof(OptionContext)}.{nameof(OptionContext.Option)} is null.");
}

if (index >= C.Option.MaxValueCount)
@@ -305,8 +305,8 @@ private void AssertValid(int index)
index >= Values.Count)
{
throw new OptionException(string.Format(
C.OptionSet.MessageLocalizer("Missing required value for option '{0}'."), C.OptionName),
C.OptionName);
C.OptionSet.MessageLocalizer($"Missing required value for option '{C.OptionName}'.")),
C.OptionName);
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.