current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I have a sequence of lines:

BinaryReader br;
[...]

//for reading a row from binary file.

MyObj[] retval = new MyObj[count];
br.BaseStream.Seek(
    beginSeek + 
    sizeof(Int16) * offsetX + 
    (offsetY + count - 1) * dataPerLine * sizeof(Int16), 
    SeekOrigin.Begin);

for (int x = 0; x < count; x++)
{
    retval[x] = new MyObj();
    retval[x].prop = br.ReadInt16();
}


//for reading a column from binary file.

MyObj[] retval = new MyObj[count];
for (int y = 0; y < count; y++)
{
    br.BaseStream.Seek(
        beginSeek + 
        sizeof(Int16) * (offsetX + count - 1) + 
        (offsetY + y) * dataPerLine * sizeof(Int16), 
        SeekOrigin.Begin);

    retval[y] = new MyObj();
    retval[y].prop = br.ReadInt16();
}

How can I improve this? It finishes run in lots of seconds. Maybe you suggest LINQ, etc.

share|improve this question

closed as off-topic by Malachi, syb0rg, Mat's Mug, rolfl♦ Jun 11 at 0:04

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example." – Malachi, syb0rg, Mat's Mug, rolfl
If this question can be reworded to fit the rules in the help center, please edit the question.
    
My hunch is you're IO bound due to all of those Seek() calls. Any way to do it without seeking will undoubtedly be faster. Read everything and keep what you need. –  Snowbody Jun 10 at 13:08
    
What is taking a lot of time? Are you reading the whole file at once or performing many individual reads? Is it feasible to read the whole file into memory in one go and then address the rows/columns that you need? –  AlanT Jun 10 at 13:10
    
I don't think this is all the code, there must be something in between these two pieces of code. you are using the same variable in both and initializing it as an object twice, I don't think we are getting the whole picture here. –  Malachi Jun 10 at 13:53
    
Yeah, it's something like a pseudo code. Just saying what I want to say with code. –  Enes Unal Jun 10 at 14:41
add comment

1 Answer

up vote 2 down vote

First off, what you have doesn't work reliably. The documentation for the BinaryReader.BaseStream property says:

"Using the underlying stream while reading or while using the BinaryReader can cause data loss and corruption. For example, the same bytes might be read more than once, bytes might be skipped, or character reading might become unpredictable."

The binary reader is buffered. When you step around in the base stream the binary reader isn't aware of that, and will read from previously buffered bytes instead of reading from the correct position in the stream. It works in the first case, where you seek before using the binary reader, but once you start using the binary reader you should leave the base stream alone.

You should use the FileStream directly rather than using a BinaryReader, at least for the second case. A file stream is seekable, a reader is not.

share|improve this answer
add comment

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