patterncsharpMinor
Comment formatter
Viewed 0 times
commentformatterstackoverflow
Problem
private string FormatComments(string comments)
{
//If comments has no space and inserted as a single line then following code will break
//the comments into multiple lines.
StringBuilder sb = new StringBuilder();
int c = 65;
int l = comments.Length;
if (l comments.Length)
break;
string indexVal = comments[index + c].ToString();
sb = sb.AppendLine(comments.Substring(index, c) + (indexVal == " " ? "" : "- "));
index = index + c;
}
if(r > 0)
sb = sb.AppendLine(comments.Substring(index, r));
return sb.ToString();
}private void SaveValuesInViewState()
{
string[] roomStr = this.RoomsString.Split('~');
List rooms = new List();
Room room = null;
foreach (string str in roomStr)
{
room = new Room();
string[] vals = str.Split('|');
//
room.NoOfRooms = Convert.ToInt32(vals[0]);
room.Type = (RoomType)Enum.Parse(typeof(RoomType), vals[1]);
room.NumberOfCots = Convert.ToInt32(vals[2]);
string[] childInfo = vals[3].Split('^');
if (Convert.ToInt32(childInfo[0]) > 0)
{
foreach (string age in childInfo[1].Split(','))
{
room.ChildrenAges.Add(Convert.ToInt32(age));
}
}
rooms.Add(room);
}
this.Rooms = rooms;
}Solution
1) You're introducing too many variables, understanding the code tends to be less easy more variables (useless variables) you have and more mutable they are. In your first method I would remove
2) Following condition seems to be useless, I can't imagine any reasonable values for
3) Use less crypty names for variables. We do not do math here. Why
4) Why
5) Do not introduce
6) You use overcomplicated format in for rooms in second method, especially around child ages. Right now you have something like this:
l and r variables. Instead of l you can simply use comments.Length and r is not that needed (see next point). I would also remove lines and i iterator, index can be used instead: for (int index = 0 ; index <= comments.Length ; index += c) { }2) Following condition seems to be useless, I can't imagine any reasonable values for
comments.Length and c which would make this condition to be true keeping in mind that we have already checked that l >= c: int r = l % c;
int lines = (l - r) / c;
if (lines == 0)
return comments;3) Use less crypty names for variables. We do not do math here. Why
c? I can guess it probably but I won't like to. 4) Why
string here? It is definitely a char:string indexVal = comments[index + c].ToString();5) Do not introduce
room variable in second method outside of loop. It is used only inside so it should be defined there. 6) You use overcomplicated format in for rooms in second method, especially around child ages. Right now you have something like this:
5|1|3|0~5|1|3|2^8,10. You have 4 different separators here, but I think ^ can be safely removed. Just put there ages if you have any or leave empty you have no children ages. You should be definitely able to parse this instead: 5|1|3|~5|1|3|8,10.Code Snippets
for (int index = 0 ; index <= comments.Length ; index += c) { }int r = l % c;
int lines = (l - r) / c;
if (lines == 0)
return comments;string indexVal = comments[index + c].ToString();Context
StackExchange Code Review Q#2426, answer score: 8
Revisions (0)
No revisions yet.