4
\$\begingroup\$

I am programming a language interpreter in C# and have recently created a set of functions that receive either a string or a string[] and split it by a received string.

For example, with a string:

"Hey:123:hello:456"
":"

It will return this array:

{"hey","123","hello","456"}

And with a string[]:

{"a:b","c","d:e:f"}
":"

It will return this array:

{"a","b","c","d","e","f"}

How can I improve efficiency and maybe make it tidier?

public string[] SplitRows(object thevar, string delimiter) {

            if (delimiter == "<newline>") delimiter = "\n";

            if (thevar.GetType() == typeof(string)) {
                string temp = (string) thevar;
                return temp.Split(new string[] {
                    delimiter
                }, System.StringSplitOptions.None);
            }
            if (thevar.GetType() == typeof(string[])) {
                return stringarraysplitter((string[]) thevar, delimiter);
            }
            return null;
        }


        public string[] stringarraysplitter(string[] arr, string delimiter) {
            string[][] tempr = new string[arr.Length][];
            for (int i = 0; i < arr.Length; i++) {
                tempr[i] = arr[i].Split(new string[] {
                    delimiter
                }, System.StringSplitOptions.None);
            }
            System.Collections.Generic.List < string > templist = new System.Collections.Generic.List < string > ();
            for (int i = 0; i < tempr.Length; i++) {
                for (int j = 0; j < tempr[i].Length; j++) {
                    templist.Add(tempr[i][j]);
                }
            }

            return templist.ToArray();
        }

Side notes:

  • SplitRows() is the only main function allowed to receive either a string array or string. Even though I use 2 functions, I cannot create 2 for the 2 variable types.
  • <newline> is a replacement string for \n in my language and can be ignored.
\$\endgroup\$
4
\$\begingroup\$
if (delimiter == "<newline>") delimiter = "\n";

Why do you need this? Why not use normal strings?

if (thevar.GetType() == typeof(string)) {

What's wrong with normal method overloading?

if (thevar.GetType() == typeof(string)) {...}
if (thevar.GetType() == typeof(string[])) {...}
return null;

If arguments is not valid throw an ArgumentException. Don't say "I'll just return null, and see where it will go wrong."

temp.Split(new string[] {delimiter}, System.StringSplitOptions.None);

This bit repeats, and should be extracted into a method.

for (int i = 0; i < tempr.Length; i++) {
    for (int j = 0; j < tempr[i].Length; j++) {

Use Linq to simplify loop operations.

Here is the cleaned up version:

    public IEnumerable<string> Split(string str, string delimiter)
    {
        return str.Split(new[]{delimiter}, StringSplitOptions.None);
    }

    public IEnumerable<string> Split(IEnumerable<string> arr, string delimiter) {
        return arr.SelectMany(s => Split(s, delimiter));
    }
\$\endgroup\$
5
  • \$\begingroup\$ Im sorry for <newline> its a replacement string for \n \$\endgroup\$ Dec 21 '15 at 8:38
  • \$\begingroup\$ I added return null because the ide asked me too return anything,in my language no other option but string or string[] could ever be passed to it \$\endgroup\$ Dec 21 '15 at 8:55
  • \$\begingroup\$ If you replace return null with throw new ArgumentException("string or string[] only") IDE (that is compiler) will not complain. \$\endgroup\$ Dec 21 '15 at 8:58
  • \$\begingroup\$ Could you explain to me,or give me some resourceful links or information on how the 2 last functions you wrote are written? I have never use ienumerable.. \$\endgroup\$ Dec 22 '15 at 6:27
  • \$\begingroup\$ See Getting Started with LinQ in C# Consult the Msdn site further as you need to learn about delegates, extension methods, IEnumerable interface etc as you are learning LinQ. \$\endgroup\$ Dec 22 '15 at 7:24
1
\$\begingroup\$

The method stringarraysplitter()'s name doesn't follow the NET naming guideline which states method names should be made out of a verb or a verb-phrase and the method should be name using PascalCase casing.
A better name would be SplitStringArray().

Because you already use a List<string> you could take advantage of the AddRange() method which makes the use of the nested loops redundant and omits the need to have a jagged array at all.

Because the method is public you should check the arguments against null.

By using the var type it will become more readable.

The placing of the opening brace { is different to what a C# developer is used to. In C# we usually place the opening brace on a new line.

Like so

public string[] stringarraysplitter(string[] arr, string delimiter) 
{
    if (arr == null || arr.Length == 0) { return arr; }
    if (delimiter == null) { delimiter = "\n"; }

    var results = new System.Collections.Generic.List<string>(arr.Length);

    for (int i = 0; i < arr.Length; i++) 
    {
        results.AddRange(arr[i].Split(new string[] {delimiter}, System.StringSplitOptions.None));
    }

    return results.ToArray();
}
\$\endgroup\$
1
  • \$\begingroup\$ I love that addrange() beautiful! \$\endgroup\$ Dec 21 '15 at 8:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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