Concatenate strings (originally binary) in 8-character chunks, 2 per row Here is the code I would like reviewed:

int lines = 4; //Example
string input = "81111111822222228333333384444444" //Example

//Use a StringBuilder to make it easier when producing the formatted code
var stringBuilder = new StringBuilder();

//Loop through the ASM Hex code and extract every 8 character and put them in 2 by 2 rows
//Also add a NOP as needed depending if it's Odd or Even
for (int i = 0; i &lt; lines; i++)
{
    string tempString = input.Substring(i * 8, 8);

    if (i % 2 == 1)
        stringBuilder.AppendLine(tempString);
    else
        stringBuilder.Append(tempString + " ");
}

//End Code with "000000" or "60000000 00000000" if empty line, needed for Gecko
if (lines % 2 == 1)
    stringBuilder.Append("00000000").AppendLine();
else
    stringBuilder.Append("60000000 00000000").AppendLine(); //This would be True in this Example (4 = Equal number)

    //Return the result
    return stringBuilder.ToString();

//stringBuilder.ToString() = 
//           "81111111 82222222
//            83333333 84444444
//            60000000 00000000"


What I am doing is basically what the comments says. What might be worth knowing is that originally the string that I want to format is read from a file as hex.

//"hex" is "input" in the first part.
var ba = File.ReadAllBytes($"temp{i}.o");

string hex = BitConverter.ToString(ba).Replace("-", "");
hex = hex.Substring(104, lines * 8); //Remove Garbage data and read only what we want (8 in length * rows of ASM)


I'm not sure if there is a more efficient way of handling the data more directly via that.

I don't think my approach now is bad, but it feels a bit "overkill". It feels like I could probably just read the data I want directly and split up the bytes or something. Sadly, I don't have the skills to know if that is something one should be doing in this case.

lines is an integer which contains the rows from a text. It's simply used for the formatting as one line = 8 characters of the wanted text (hex).

An example of hex text would be something like:

81111111822222228333333384444444855555558666666687777777


It's simplified so it's easier to tell (each Code starts with 8 in this example).

What I want it to turn into in this case would be:

81111111 82222222
83333333 84444444
85555555 86666666
87777777 00000000


And if (8xxxxxxx) actually took up both 2 lines in the row, I would add:

60000000 00000000


The original code is of no interested as it's completely different (Assembly). The only useful part is the rows of code because one row of Assembly = 8 hex characters.
You need to separate splitting the string into bytes from creating lines. 

By following the Single Responsibility Principle it's much easier to optmize your code because you can focus on one unit at a time. Other benefits are: easier maintenance and easier testing.

(I call it just byte because it's 8 elements long)

Ok, so here you'll need two methods. The first one just creates a list of strings and can be greatly simplified because we can add each substring directly to the list.

public static IEnumerable&lt;string&gt; Bytify(this string value)
{
    var byteLength = 8;
    for (var i = 0; i &lt; value.Length; i += byteLength)
    {
        yield return value.Substring(i, byteLength);
    }       
}


There are many other methods for splitting the string into batches but that's not the point. It's about the SRP. If you have separate methods you can exchange the algorithm for another one without breaking or adjusting everything else. The method creating the lines will still work because it doesn't need to know how you create the list, it just requires one.

Done. Now you can work with the bytes.



The other method can build the lines. With some LINQ it's very easy to create each one. Notice that I also flipped the condition to be values.Count % 2 == 0 that I find is easier to understand.

public static string ToBytePairs(this IList&lt;string&gt; values)
{
    var text = new StringBuilder();
    var lineWidth = 2;
    var byteSeparator = " ";        

    for (var i = 0; i &lt; values.Count; i += lineWidth)
    {
        text.AppendLine(string.Join(byteSeparator, values.SkipFast(i).Take(lineWidth)));
    }

    if (values.Count % 2 == 0)
    {
        text.AppendLine(string.Join(byteSeparator, new[] { "60000000", "00000000" }));
    }
    else
    {
        // There is line break at the and already so we need to insert this one before it.
        text.Insert(text.Length - 1, $"{byteSeparator}00000000");
    }

    return text.ToString();
}


As a bonus a faster skip extension so that the entire list doesn't have to be enumerated.

private static IEnumerable&lt;T&gt; SkipFast&lt;T&gt;(this IList&lt;T&gt; values, int index)
{
    for (var i = index; i &lt; values.Count; i++)
    {
        yield return values[i];
    }
}


The default Skip is not a super fast solution but it's enough for short lists. You can easily find Skip optimized for IList that uses indexes if your strings a very big and every second matters. Just don't optimize too much if you don't need to. Premature optimization is the root of all evil.

Example:

var input = "8111111182222222833333338444444499999999";
var bytes = input.Bytify().ToList();
bytes.ToBytePairs().Dump(); // Dump() is LINQPad's method.


Result



// 8111111182222222833333338444444499999999

81111111 82222222
83333333 84444444
99999999 00000000


or

// 81111111822222228333333384444444

81111111 82222222
83333333 84444444
60000000 00000000