[RESOLVED] StringManipulation

Hi guys,

I just want to make sure that what I am doing here is correct. The following class splitting a string into two strings when it finds ",". It works fine but I am a bit worried of my code.
Can you guys please let me know is it correct way splitting the string or is there any easier way I can write. I appreciate your comments.

public class StringManipulation
{

//constructor
StringManipulation(string textValues)
{

this.textOne = this.GetTextOne(textValues);
this.textTwo = this.GetTextTwo(textValues);
}
//Methods
public string GetTextOne(string textValues)
{
string[] arInfo = new string[1000];
string info = textValues;
char[] splitter = {','};
arInfo = info.Split(splitter);
if(arInfo.Length > 0)
{
return arInfo[0].Trim();
}
else
{
return string.Empty;

}


}
public string GetTextTwo(string textValues)
{
string[] arInfo = new string[1000];
string info = textValues;
char[] splitter = {','};
arInfo = info.Split(splitter);
if(arInfo.Length > 1)
{
return arInfo[1].Trim();
}
else
{
return arInfo[0].Trim();

}

}
//propterties
public string TextOne
{

return this.textOne;
}
public string TextTwo
{
return this.textTwo

}
//Member variables
private string textOne;
private string textTwo;

}

Creating the arrays of strings and such would be a lot of processing for no real return. If all you are doing is splitting a string into two, then the following code is much more concise. (but still isn't necessarily the best)

Code:

---

using System;

public class StringManipulation
{
  //constructor
  StringManipulation(string textValues)
  {
    this.textOne = this.GetTextOne(textValues);
    this.textTwo = this.GetTextTwo(textValues);
  }
  //Methods
  public string GetTextOne(string textValues)
  {
    int index = textValues.IndexOf(",");
    return textValues.Substring(0, index);
  }
  public string GetTextTwo(string textValues)
  {
    int index = textValues.IndexOf(",");
    return textValues.Substring(index+1, (textValues.Length - index));
  }
  //propterties
  public string TextOne
  {
    get
    {
      return this.textOne;
    }
  }
  public string TextTwo
  {
    get
    {
      return this.textTwo;
    }
  }
  //Member variables
  private string textOne;
  private string textTwo;

  // added just to test it out...
  public static void Main()
  {
    string MyString = "1234567890,abcdefghijklmnopqrstuvwxyz";

    StringManipulation NewString = new StringManipulation(MyString);

    Console.WriteLine("Part 1: >>{0}<<", NewString.TextOne );
    Console.WriteLine("Part 2: >>{0}<<", NewString.TextTwo );
    Console.Read();
  }
}

---

I based this code off of what you had already done.

Brad!

Code:

---

StringManipulation(string textValues)
  {
    int index = textValues.IndexOf(",");

    this.textOne = textValues.Substring(0, index);
    this.textTwo = textValues.Substring(index + 1);
  }

---

Then you don't need GetTextOne() or GetTextTwo().

Very good. I was focused on just getting his class to work and should have looked closer....

Brad!

Bravo! In fact I did not know that I can use as simple that,

Thanks very much guys. By the way Brad_jones it is not his. it is her ;)

Again how do I say that this question is [RESOLVED]. Do I need to edit my post and modify the subject as [RESOLVED].

bye for now.

OOPS! sorry it did not actually work wossname.

It throws an errrrrrrrrrror

Length cannot be less than zero. Parameter name: length

it always expect sometext after "," but my class functions handle that.


;)

Quote:

---

Originally Posted by dummy_pmgr

Again how do I say that this question is [RESOLVED]. Do I need to edit my post and modify the subject as [RESOLVED].

---

Nearly on the top of this page, there is some links. One of them says "thread tools", if you click it, then you will get a pop up menu. Press the resolved link, and it will be done for you..:)



PS: I am a bit fussed by your last post. Does it work now, or do you still have the error?


- ØØ -

Hi NoteMe,

Thanks for Thread tools.

Yea! it works fine according to my post. I wanted to simplyfy my code. If you go through this whole thread you will understand what I mean.

thanks

Quote:

---

Originally Posted by dummy_pmgr

OOPS! sorry it did not actually work wossname.

It throws an errrrrrrrrrror

Length cannot be less than zero. Parameter name: length

it always expect sometext after "," but my class functions handle that.


;)

---

It works perfectly, your data must be bad. You get an index of -1 when IndexOf() did not find a comma in your string :)

Thanks wossname, I will try with some more data and let you know.

cheers,

Hello again Wossname,

It is all my dodgy data. It works great and perfectly now.

I changed a bit to handle the empty data.

StringManipulation(string textValues)
{
int index = textValues.IndexOf(",");
this.textOne = textValues.Substring(0, index);

if(index != -1)
{
this.textTwo = textValues.Substring(index + 1);
}
else
{
this.textTwo = "There is no string after comma";
}
}

One final question in this thread which I really dont know. Is it the best practice to write a lots of code in the constructor?

Thanks for your help.

dummy

"Lots of code" is misleading. It is best to do as much of work as possible in the constructor, because it will save having to do it later on when speed might be critical.

There are hundreds of ways to maximise performance and this is just one of them, but don't go overboard. You should be able to justify every line of code you write.

Thanks wossname