Ethan Strauss wrote:
Quote:
"Jeroen Mostert" wrote:
>
Quote:
>Ethan Strauss wrote:
Quote:
>>Hi,
>> I have written a generic method which does different things depending on
>>the type of the parameter. I got it to work, but it seems really inelegant.
>>Is there a better way to do this?
>> In the code below, ColumnMap is a simple struct which basically has three
>>properties, Header (a string), Index (an int), TypeOfData (which is a
>>DataType which is a local eNum). _Mapping is a local List<ColumnMap>.
>>>
>>>
>> public ColumnMap GetMap<T>(T value)
>> {
>> Type ThisType = typeof(T);
>> foreach (ColumnMap ThisMap in _Mapping)
>> {
>> if (ThisType == typeof(string))
>> {
>> if (ThisMap.Header.ToUpper() ==
>>value.ToString().ToUpper())
>Don't do this, use one of the overloads of String.Equals() instead.
Why not? Not that I object to String.Equals(), but I would think that the
amount of extra work the CPU has to do would be pretty trivial. Am I wrong?
>
The problem is that if you use .ToUpper(), it's not clear what sort of
comparison you're intending. The default comparison is culture-specific,
which is rarely appropriate and leads to such classical problems as the one that
"i".ToUpper() == "I".ToUpper()
is *false* when the locale is Turkish (the so-called "Turkish i problem"),
because Turkish has two distinct letter i's (with and without dot).
String.Equals() avoids these issues. So do .ToUpper() with an appropriate
argument and .ToUpperInvariant(), but if you have the opportunity to do an
efficient comparison instead of producing more strings, why waste it? It's
still one line of code.
Quote:
Quote:
>Note also that it's poor form to make a function that's nominally intended
>to get an existing item create a new one when it can't find the value
>supplied. Are clients supposed to check for existence first if they want to
>find out if the value is present at all, or do default ColumnMaps have some
>sort of .IsActuallyNotValid property?
>
I do expect that clients will check for existence first, but if I leave this
out, then I get a "not all paths return a value" type error. Would you prefer
throwing an error here? If so, why?
>
There are plenty of patterns here:
- Provide GetValue() and have it return null if the specified value isn't
there. For this to work, null must not be a valid item in the collection.
This is by far the easiest approach. Clients have little opportunity for
failing by not checking the return value, because any call on the resulting
object will throw a NullReferenceException. It's also easy to propagate null
values to callers upstream that use the same convention.
- As a refinement of this, you can return a "null object" instead. This
object is an actual object (not a null reference) which simply provides a
do-nothing or trivial implementations of every method. This can remove
checking of edge cases and simplify logic in some cases, but the downside is
that it complicates handling for clients that are only interested in "real"
objects. Also, some methods may simply not allow for a meaningful null
implementation.
- Provide HasValue() and GetValue() and have GetValue() throw an exception.
This works, but it means getting a value if you don't know it exists is
twice as expensive (GetValue() will presumably search for the value in
exactly the same way HasValue() does). It also means that if this collection
is used in a concurrent scenario, clients *must* lock on the collection for
every single access, because there's no atomic way of retrieving a value
successfully.
- Provide GetValue(), which throws an exception if the item is not there,
and bool TryGetValue(out value), which sets value and returns a boolean
indicating whether the item was found. This is the generalization of
solution #1, used by (among others) Dictionary<T>. This works even if null
is an item in the collection, but TryGetValue() is cumbersome to call, and
this can get annoying especially if the common case is *not* knowing whether
an item is present.
Which one to use depends on your scenario. There's another approach that,
while often seen, I would rarely consider useful:
- Provide only GetValue() and have it throw an exception for values that
aren't there. This is only appropriate if clients *must* know a value exists
and any failure to know this is to be considered a bug, because "exceptions
should be exceptional". The question to ask here is: how can clients
*definitely know* the item is there unless they know someone else who put it
there? If they do, why didn't that party simply pass them the item instead
of making them retrieve it?
--
J.
http://symbolsprose.blogspot.com 
