current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite
1

I have an extension that implements all the BinaryReader/BinaryWriter in the Stream class. I want to create generic methods for them. Is there a nicer way of doing this?

public static void Write<T>(this Stream stream, T num)
{
    if (num is byte)
    {
        stream.Write((byte)(object)num);
    }
    else if (num is Int16)
    {
        stream.Write((Int16)(object)num);
    }
    else if (num is Int32)
    {
        stream.Write((Int32)(object)num);
    }
}

public static T Read<T>(this Stream stream)
{
    object ret = null;

    if (typeof(T) == typeof(byte))
        ret = stream.ReadInt8();
    else if (typeof(T) == typeof(Int16))
        ret = stream.ReadInt16();
    else if (typeof(T) == typeof(Int32))
        ret = stream.ReadInt32();

    if (ret == null)
        throw new ArgumentException("Unable to read type - " + typeof(T));
    return (T)ret;
}

Now using this for the Read method.

static Dictionary<Type, Func<Stream, object>> ReadFuncs = new Dictionary<Type, Func<Stream, object>>()
{
    {typeof(bool), s => s.ReadBoolean()},
    {typeof(byte), s => s.ReadInt8()},
    {typeof(Int16), s => s.ReadInt16()},
    {typeof(Int32), s => s.ReadInt32()},
    {typeof(Int64), s => s.ReadInt64()},
    {typeof(string), s => s.ReadString()},
};
public static T Read<T>(this Stream stream)
{
    if (ReadFuncs.ContainsKey(typeof(T)))
        return (T)ReadFuncs[typeof(T)](stream);

    throw new NotImplementedException();
}
share|improve this question

2 Answers 2

up vote 5 down vote accepted

I'm guessing you defined more write methods than this? Otherwise the Write() would call itself recursively infinitely. You probably have overloads for every supported type, making the generic write method superfluous. This would also explain the weird casting you are doing. I'm guessing an overloaded function with a specific type has precedence over a generic method, which is why it could work. This however, doesn't make it a clean solution.

Your Read() method seems to be more useful. Instead of throwing a ArgumentException I would prefer throwing a NotSupportedException.

As a sidenote, are you sure you want to make these extension methods of Stream and not some more specific stream? Stream can be any type of stream.

In what way is this an improvement over using BinaryWriter/BinaryReader? For the purposes I gather from your code I would only create a Read<T> extension method for BinaryReader.

If you feel like experimenting a bit, there might be a more performant/cleaner solution, although you'll have to benchmark to be sure. You could create a Dictionary<Type, Func<object>> to hold a matching 'reader' for every type. Initializing the delegates to store in this dictionary is the tough part, on which information can be found in this article. Most likely this is not worth the effort, although it still is a very interesting article. :) Be sure to also check out the same solution implemented by using Expression Trees in the comments of the article. For a similar implementation example I did, check out this question I posted on Code Review.

share|improve this answer
    
Well BinaryReader/Writer both take a Stream so I figured I would make the extension for Stream. The Write is really just because I want to do Write<byte>(5) instead of Write((byte)5). Is there any nicer way to do the Read function though? –  Will Mar 28 '11 at 12:53
    
@High: I updated my answer with a possible alternate solution, although for the small amount of supported types from BinaryReader, your current implementation is probably better. –  Steven Jeuris Mar 28 '11 at 13:18
    
I don't think I will end up using the Write generic but I still created an extension for regular Write. Reason is because it does not require you to create an instance of an object to read from a stream. Also BinaryReader/Writer both Close the stream when they are disposed which I do not like. I updated my question with what I am assuming your second suggestion is. –  Will Mar 29 '11 at 0:26
    
@High: Ow .. that makes a lot more sense, .. i was trying to make things too complicated. ;p Yes that is along the lines I meant. –  Steven Jeuris Mar 29 '11 at 9:47
up vote 1 down vote

I know your trying to simplify your code, but I think it will be better to go the opposite way, avoid generic types, and doing specific type procedures:

public static void WriteByte(this Stream stream, byte num) { ... }
public static void WriteInt16(this Stream stream, Int16 num) { ... }
public static void WriteInt32(this Stream stream, Int32 num) { ... }

Why. Because its very easy to call the wrong method, calling "Write(<int32>)" instead of "Write(<byte>)". And in some scenario, the developer may not treat those integer types as the same.

share|improve this answer
    
The generics are separate from the other Write methods so I am not worried about calling the wrong one. Also WriteByte is not possible due to the fact that Stream already has a method by that name that returns an integer. –  Will Mar 29 '11 at 0:06

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.