Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");
deleted 474 characters in body
Source Link
t3chb0t
  • 44.6k
  • 9
  • 84
  • 190

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");

Lastly, the extension that creates a dictionary doesn't need to be an extension at all because the dictionary's constructor can take an IEnumerable as a parameters so this:

public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> keyValuePairs)
{
 return keyValuePairs.ToDictionary(kv => kv.Key, kv => kv.Value);
}

is the same as

 new Dictionary<TKey, TValue>(items);

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");

Lastly, the extension that creates a dictionary doesn't need to be an extension at all because the dictionary's constructor can take an IEnumerable as a parameters so this:

public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> keyValuePairs)
{
 return keyValuePairs.ToDictionary(kv => kv.Key, kv => kv.Value);
}

is the same as

 new Dictionary<TKey, TValue>(items);

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");
added 215 characters in body
Source Link
t3chb0t
  • 44.6k
  • 9
  • 84
  • 190

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");

Lastly, the extension that creates a dictionary doesn't need to be an extension at all because the dictionary's constructor can take an IEnumerable as a parameters so this:

public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> keyValuePairs)
{
 return keyValuePairs.ToDictionary(kv => kv.Key, kv => kv.Value);
}

is the same as

 new Dictionary<TKey, TValue>(items);

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

Lastly, the extension that creates a dictionary doesn't need to be an extension at all because the dictionary's constructor can take an IEnumerable as a parameters so this:

public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> keyValuePairs)
{
 return keyValuePairs.ToDictionary(kv => kv.Key, kv => kv.Value);
}

is the same as

 new Dictionary<TKey, TValue>(items);

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
 this Dictionary<TKey, TValue> dictionary, 
 TValue value, 
 params TKey[] keys)
{
 foreach (var key in keys) dictionary.Add(key, value);
 return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
 new Dictionary<string, string>()
 .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");

Lastly, the extension that creates a dictionary doesn't need to be an extension at all because the dictionary's constructor can take an IEnumerable as a parameters so this:

public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> keyValuePairs)
{
 return keyValuePairs.ToDictionary(kv => kv.Key, kv => kv.Value);
}

is the same as

 new Dictionary<TKey, TValue>(items);
Source Link
t3chb0t
  • 44.6k
  • 9
  • 84
  • 190
Loading
lang-cs

AltStyle によって変換されたページ (->オリジナル) /