patterncsharpModerate
Removing bytes from a byte array
Viewed 0 times
removingarraybytebytesfrom
Problem
I am reading from a
I initially remove a group of know bytes using:
Then I have the following method to search for a pattern with a wilcard in a byte array:
```
public static int FindPattern(byte[] body, byte[] pattern, bool[] wild, int start = 0)
{
var foundIndex = -1;
if (body.Length body.Length - pattern.Length || pattern.Length > body.Length) return foundIndex;
for (var index = start; index <= body.Length - pattern.Length; index += 4)
if (wild[0] || (body[index] == pattern[0]))
{
var match = true;
for (var index2 = 1; index2 <= pattern.Length - 1; index2++)
{
if (wild[index2] || (body[index + index2] == pattern[index2])) continue;
match = false;
break;
}
if (!match) continue;
foundIndex = index;
break;
}
Networkstream, and the response code is:private async void GetResponse(NetworkStream stream)
{
while (true)
{
var readBuffer = new byte[4096];
var asyncReader = await stream.ReadAsync(readBuffer, 0, readBuffer.Length);
var result = RemoveBytes(readBuffer, new byte[] { 130, 119, 126, 118, 48, 49, 12, 19, 14 });
List l = null;
var w = 1;
while (w > 0)
{
w = FindPattern(result, new byte[] { 1, 0, 131 }, new[] { false, true, false });
if (w <= 0) continue;
l = result.ToList();
l.RemoveAt(w);
l.RemoveAt(w);
l.RemoveAt(w);
Buffer.BlockCopy(l.ToArray(), 0, result, 0, result.Length - 3);
}
if (l != null) txtBoxMessagesIn.Text += Encoding.ASCII.GetString(l.ToArray()) + Environment.NewLine;
if (asyncReader <= 0) break;
}
}I initially remove a group of know bytes using:
public static byte[] RemoveBytes(byte[] input, byte[] pattern)
{
if (pattern.Length == 0) return input;
var result = new List();
int i;
for (i = 0; i input[i + j] != t).Any();
if (foundMatch) i += pattern.Length - 1;
else result.Add(input[i]);
}
for (; i < input.Length; i++)
{
result.Add(input[i]);
}
return result.ToArray();
}Then I have the following method to search for a pattern with a wilcard in a byte array:
```
public static int FindPattern(byte[] body, byte[] pattern, bool[] wild, int start = 0)
{
var foundIndex = -1;
if (body.Length body.Length - pattern.Length || pattern.Length > body.Length) return foundIndex;
for (var index = start; index <= body.Length - pattern.Length; index += 4)
if (wild[0] || (body[index] == pattern[0]))
{
var match = true;
for (var index2 = 1; index2 <= pattern.Length - 1; index2++)
{
if (wild[index2] || (body[index + index2] == pattern[index2])) continue;
match = false;
break;
}
if (!match) continue;
foundIndex = index;
break;
}
Solution
You aren't using braces
If you decide not to use them, you should stick to your style. Right now you are mixing them.
Using a guard clause is a good idea, but if one needs to scroll to the right to see each condition its suboptimal.
Use some vertical space to group related code together makes your code much more readable and easier to maintain.
By adding a method which returns an
The name of the method could/should be adjusted but I couldn't come up with a better one.
If we would now add a method to check if there really is a pattern like so
we can then implement the mentioned points and change the former
So let us take a look at this
-
private async void GetResponse(NetworkStream stream)
{
by
{} for the outer loops body which is bad, bad, bad. You should always use them although they are optional it just will make your code less error prone. If you decide not to use them, you should stick to your style. Right now you are mixing them.
Using a guard clause is a good idea, but if one needs to scroll to the right to see each condition its suboptimal.
Use some vertical space to group related code together makes your code much more readable and easier to maintain.
By adding a method which returns an
IEnumerable containing the start positions of the possible patterns will improve readability like so private static IEnumerable GetPatternStartings(byte[] body, byte[] pattern, bool[] wild, int start = 0)
{
for (var index = start; index <= body.Length - pattern.Length; index += 1)
{
if (wild[0] || (body[index] == pattern[0]))
{
yield return index;
}
}
}The name of the method could/should be adjusted but I couldn't come up with a better one.
If we would now add a method to check if there really is a pattern like so
private static bool IsPattern(byte[] body, byte[] pattern, bool[] wild, int bodyIndex)
{
for (var index = 1; index <= pattern.Length - 1; index++)
{
if (wild[index] || (body[bodyIndex + index] == pattern[index])) continue;
return false;
}
return true;
}we can then implement the mentioned points and change the former
FindPattern() method to public static int FindPattern(byte[] body, byte[] pattern, bool[] wild, int start = 0)
{
var foundIndex = -1;
if (body.Length body.Length - pattern.Length
|| pattern.Length > body.Length)
{ return foundIndex; }
foreach (int patternStart in GetPatternStartings(body, pattern, wild, start))
{
if (IsPattern(body, pattern, wild, patternStart)) { return patternStart; }
}
return foundIndex;
}So let us take a look at this
private async void GetResponse(NetworkStream stream)
{
while (true)
{
var readBuffer = new byte[4096];
var asyncReader = await stream.ReadAsync(readBuffer, 0, readBuffer.Length);
var result = RemoveBytes(readBuffer, new byte[] { 130, 119, 126, 118, 48, 49, 12, 19, 14 });
List l = null;
var w = 1;
while (w > 0)
{
w = FindPattern(result, new byte[] { 1, 0, 131 }, new[] { false, true, false });
if (w <= 0) continue;
l = result.ToList();
l.RemoveAt(w);
l.RemoveAt(w);
l.RemoveAt(w);
Buffer.BlockCopy(l.ToArray(), 0, result, 0, result.Length - 3);
}
if (l != null) txtBoxMessagesIn.Text += Encoding.ASCII.GetString(l.ToArray()) + Environment.NewLine;
if (asyncReader <= 0) break;
}
}- The call to
ToList()and 3 timesRemoveAt()together with theBuffer.BlockCopy()will slow down the whole thing.
-
if (w
-
the pattern and wild to be used in FindPattern() should be created at the start of the method. It won't change.
-
w is a poorly named variable, let us use patternIndex instead.
Applying this will lead to
private async void GetResponse(NetworkStream stream)
{
byte[] pattern = new byte[] { 1, 0, 131 };
bool[] wild = new bool[] { false, true, false };
int patternLength = pattern.Length;
while (true)
{
var readBuffer = new byte[4096];
var asyncReader = await stream.ReadAsync(readBuffer, 0, readBuffer.Length);
var result = RemoveBytes(readBuffer, new byte[] { 130, 119, 126, 118, 48, 49, 12, 19, 14 });
var patternIndex = 1;
while (patternIndex > 0)
{
patternIndex = FindPattern(result, pattern, wild);
if (patternIndex 0)
{
txtBoxMessagesIn.Text += Encoding.ASCII.GetString(l.ToArray()) + Environment.NewLine;
}
if (asyncReader <= 0) { break; }
}
}
So now we are finished, aren't we ? NO !
Lets check this RemoveBytes() method, because I see a pattern there.
After renaming the method to RemovePattern we can have 2 overloaded ones like so
private static byte[] RemovePattern(byte[] input, byte[] pattern)
{
return RemovePattern(input, pattern, new bool[pattern.Length]);
}
private static byte[] RemovePattern(byte[] input, byte[] pattern, bool[] wild)
{
int patternLength = pattern.Length;
var patternIndex = -1;
while ((patternIndex = FindPattern(input, pattern, wild)) >= 0)
{
byte[] current = new byte[input.Length - 3];
Buffer.BlockCopy(input, 0, current, 0, patternIndex);
Buffer.BlockCopy(input, patternIndex + patternLength, current, patternIndex, input.Length - patternIndex - patternLength);
input = current;
}
return input;
}
You see, I have extracted the removal to a separate method. Now the former GetResponse() method will become this
``private async void GetResponse(NetworkStream stream)
{
by
Code Snippets
private static IEnumerable<int> GetPatternStartings(byte[] body, byte[] pattern, bool[] wild, int start = 0)
{
for (var index = start; index <= body.Length - pattern.Length; index += 1)
{
if (wild[0] || (body[index] == pattern[0]))
{
yield return index;
}
}
}private static bool IsPattern(byte[] body, byte[] pattern, bool[] wild, int bodyIndex)
{
for (var index = 1; index <= pattern.Length - 1; index++)
{
if (wild[index] || (body[bodyIndex + index] == pattern[index])) continue;
return false;
}
return true;
}public static int FindPattern(byte[] body, byte[] pattern, bool[] wild, int start = 0)
{
var foundIndex = -1;
if (body.Length <= 0
|| pattern.Length <= 0
|| start > body.Length - pattern.Length
|| pattern.Length > body.Length)
{ return foundIndex; }
foreach (int patternStart in GetPatternStartings(body, pattern, wild, start))
{
if (IsPattern(body, pattern, wild, patternStart)) { return patternStart; }
}
return foundIndex;
}private async void GetResponse(NetworkStream stream)
{
while (true)
{
var readBuffer = new byte[4096];
var asyncReader = await stream.ReadAsync(readBuffer, 0, readBuffer.Length);
var result = RemoveBytes(readBuffer, new byte[] { 130, 119, 126, 118, 48, 49, 12, 19, 14 });
List<byte> l = null;
var w = 1;
while (w > 0)
{
w = FindPattern(result, new byte[] { 1, 0, 131 }, new[] { false, true, false });
if (w <= 0) continue;
l = result.ToList();
l.RemoveAt(w);
l.RemoveAt(w);
l.RemoveAt(w);
Buffer.BlockCopy(l.ToArray(), 0, result, 0, result.Length - 3);
}
if (l != null) txtBoxMessagesIn.Text += Encoding.ASCII.GetString(l.ToArray()) + Environment.NewLine;
if (asyncReader <= 0) break;
}
}private async void GetResponse(NetworkStream stream)
{
byte[] pattern = new byte[] { 1, 0, 131 };
bool[] wild = new bool[] { false, true, false };
int patternLength = pattern.Length;
while (true)
{
var readBuffer = new byte[4096];
var asyncReader = await stream.ReadAsync(readBuffer, 0, readBuffer.Length);
var result = RemoveBytes(readBuffer, new byte[] { 130, 119, 126, 118, 48, 49, 12, 19, 14 });
var patternIndex = 1;
while (patternIndex > 0)
{
patternIndex = FindPattern(result, pattern, wild);
if (patternIndex < 0) { break; }
byte[] current = new byte[result.Length - patternLength];
Buffer.BlockCopy(result, 0, current, 0, patternIndex);
Buffer.BlockCopy(result, patternIndex + patternLength, current, patternIndex, result.Length - patternIndex - patternLength);
result = current;
}
if (result != null && result.Length > 0)
{
txtBoxMessagesIn.Text += Encoding.ASCII.GetString(l.ToArray()) + Environment.NewLine;
}
if (asyncReader <= 0) { break; }
}
}Context
StackExchange Code Review Q#106118, answer score: 10
Revisions (0)
No revisions yet.