3
\$\begingroup\$

Good day. My code works but I think there is a better way to do this. Like string pattern or string manipulation. I'm not yet familiar with both terms.

The goal is to get "=A1-A2-B3-D4-WHATEVER STRING=". From array of strings.

The code is :

string[] arr = { "A1", "A2", "B3", "D4", "WHATEVER STRING"}; // can be any string value
string newString = "=";
for (int i = 0; i < arr.Length; i++) {
    if (i == arr.Length-1) {
        newString  += arr[i].ToString() + "="; 
    } else {
        newString += arr[i].ToString() + "-";
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ To the reviewers: while not particularly well-written, I do think this question is simple enough that it can't be closed for Missing Review Context. Please notify me if you disagree. \$\endgroup\$ Commented Aug 27, 2020 at 1:34

3 Answers 3

Reset to default
8
\$\begingroup\$

You should use String.Join() here. Its easier to read and shorter as well.

Like

string[] arr = { "A1", "A2", "B3", "D4", "WHATEVER STRING" }; // can be any string value
string result = "=" + string.Join("-", arr) + "=";
\$\endgroup\$
2
\$\begingroup\$

Another simpler and more universal way than the original code in the OP (without using String.Join()) would be

string[] arr = { "A1", "A2", "B3", "D4", "WHATEVER STRING"}; // can be any string value
string newString = "";
foreach (string elem in arr) {
    if(newString <> "") {
        newString += "-"
    }
    newString  += elem; // ToString() is not needed in this case
}
newString = "=" + newString + "=";

I often use that pattern in the foreach (or other kind of) loop to concatenate strings with a delimiter. This works in many programming languages, which don't offer the functionality of C#'s String.Join(), or also when you need some ToString() operation for the array elements (e.g. int values).

\$\endgroup\$
2
  • 1
    \$\begingroup\$ I am the downvoter and usually do not DV. (1) I strongly disagree that this is simpler. When the framework offers a short, performant method, one should favor it. String.Join is such a method, whereas some LINQ calls are not since they are not the best perfomers. (2) If you do this extra coding, you should use StringBuilder instead of re-creating each freakin' string with each small change. \$\endgroup\$ Commented Aug 26, 2020 at 14:31
  • 1
    \$\begingroup\$ @RickDavin 1. I didn't say that this is simpler than using String.Join() (I probably should clarify that). What I meant is that it's simpler and better readable than the OPs original approach 2. As mentioned this approach is quite universal and works with many programming languages (I have to work with a great variety of PLs professionally). 3. That said, not every programming language offers a class like StringBuilder either. \$\endgroup\$ Commented Aug 26, 2020 at 14:44
2
\$\begingroup\$

You may wish to unroll the loop to avoid putting a condition inside it. We know that i == arr.Length - 1 condition is only ever going to be true once, and we know when it will happen, so there's no sense in testing for it on every loop iteration.

As a minimal change to your logic:

string[] arr = { "A1", "A2", "B3", "D4", "WHATEVER STRING"}; 
string newString = "=";

for (int i = 0; i < arr.Length - 1; i++) {
    newString += arr[i].ToString() + "-";
}

newString += arr[arr.Length - 1] + "=";
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.