StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>This code is a mess; you should probably try to clean it up. In the process, you may discover your bug gets fixed by itself.</p> <ul> <li>You have several empty catch clauses which is a big red flag (the commented out using block is a much better idea). These should <em>all</em> be removed; it's very unlikely these are a good idea.</li> <li>You've got a thread.Sleep statement which is probably superflous - and if it's not, that's a sign of a threading bug.</li> <li>You should split out basic functionality into helper methods. This increases the readability and thus debuggability of your code - and automatically provides some documentation in the form of the name of the private helper method. For instance, your read-from A write-to B code could be a method - you've duplicated this functionality in both <code>SplitFile</code> and <code>Decompress</code>.</li> <li>You have a bunch of erroneous <code>.Read(</code> statements that assume that read actually reads the full buffer - it doesn't, it waits until at least 1 byte is available and just returns what's immediately available, or returns 0 if the stream is finished. You should <em>never</em> ignore the count of bytes returned by the <code>.Read(</code> method. If you split out the (better) while-based code from <code>SplitFile</code> and <code>Decompress</code> into a helper method, you can use this elsewhere too. <strong>This is quite likely to cause problems when writing to a network or physical drive</strong>.</li> <li>multiple using blocks can be written without extra curly braces to improve readability. If you do this, VS.NET won't add an indent level for every using() clause, but just one for all of them.</li> <li>It's not quite clear to me, but it looks like you're working with a bunch of intermediate files. A cleaner (and potentially much faster) approach would be to simply work on streams and have a wrapper that happens to provide file streams.</li> <li>You don't need to <code>.Flush()</code> before <code>.Close()</code>.</li> <li>It's a good habit to put really <em>every</em> <code>IDisposable</code> in a using block. Even things like <code>MemoryStream</code> and <code>CryptoStream</code> - it's likely not to matter, but I see you're <code>.Close()</code>-ing them anyhow (so you're not saving any code), and you're violating the code-contract and you probably don't know about their implementation, so you're relying on unspecified behavior; that's not worth it.</li> <li><code>.Substring(....).ToString()</code> is equivalent to <code>.Substring()</code></li> </ul> <p>Basically, it looks like you're making a big complicated thing using technology you're not quite familiar with; try splitting it into small, clean bits that allow you to more precisely pinpoint what you need - that way you can better maintain control over any new tech.</p> <p>Clean up what you have first; then try pinpointing whatever error(s) you have remaining - if you're a bit lucky, you won't have any...</p> <p>I hope this helps!</p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload