1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36
private static void Log(object message, MessageType type) { string str = message.ToString(); //Handle newlines if (str.IndexOf('\n') >= 0) { foreach (string s in str.Split('\n')) Log(s, type); return; } //Split the string recursively in smaller strings to fit the console window for (int i = str.Length; i > 0; i--) //TODO: Optimize, adding a message of 1000 characters takes too long { if (MessageFits(str.Substring(0, i))) { messages.Add(new Message(str.Substring(0, i), type)); //If user was already all scrolled down, keep him scrolled down if (shownMessage == messages.Count - 2) shownMessage = messages.Count - 1; Log(str.Substring(i), type); //Recursive call to handle the rest break; } } } private static bool MessageFits(string message) { return font.MeasureString(message).X <= window.ClientBounds.Width - (padding * 2) - scrollBarWidth; }
Refactorings
No refactoring yet !
fairweather
December 9, 2009, December 09, 2009 16:44, permalink
// First of all, the algorithm you are using to fit the messages in the windows is quadratic,
// recursive and splits strings left right and center.
// The most obvious thing you could do is replace the (int i = str.Length; i > 0; i--) loop with
// binary chop which should bring it down to NlogN. After that, I would look into removing the
// recursion using an outer loop
while(true){
string this_line, rest;
Binary_Chop(str, out this_line, out rest);
if(rest == "")
break;
str = rest;
}
// Perhaps the most pragmatic optimization that you can implement is using a monospace font on the
// which will eliminate the need to recalculate the string width and compare it to the console
// window; you could cache the number of fitting chars in a Dictionary<int, int>, keyed by the available
// screen width. Indeed, even if you're using a non-monospace font you can get a good upper bound
// by measuring the number of 'm' characters that can fit in the screen and then using that either as
// a fixed splitting position or as a guidance for the loop (which will remove the need for binary search):
var str = "m";
while(fits(str, window_width))
str += "m";
int max_chars_per_line = str.Length - 1;
rikkus
December 13, 2009, December 13, 2009 12:16, permalink
How about a more standard word wrapping algorithm? The simple 'minimum length' algorithm mentioned here http://en.wikipedia.org/wiki/Word_wrap should suit.
James
January 19, 2010, January 19, 2010 18:40, permalink
I agree with fairweather that if at all possible you ought to use a monotype font so that you don't have to do complicated measurements. If you do that, then the code below should break the lines appropriately, and be orders of magnitude faster. If you're stuck using a "pretty" font, you could follow the same pattern, but keep a running total of the length of each character in the line so far, and check that against your maximum. Right now you're checking if "a" fits, then if "ab" fits, then if "abc" fits, etc., which means by the time you get to the end of the line, you've calculated the length of "a" a whole bunch of times. Instead, you can see how many pixels "a" takes, and the next time through add the number of pixels "b" takes, etc.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
private static void Log(object message, MessageType type) { foreach(string line in GetLines(lorem)) { Console.WriteLine(line); } } public static IEnumerable<string> GetLines(string message) { int startWord = 0; int endWord = 0; int startLine = 0; int endLine = 0; while(endLine < message.Length) { if (endWord - startLine >= MaxLength) { yield return message.Substring(startLine, endLine - startLine); startLine = endLine = startWord; } switch(message[endWord]) { case '\n': yield return message.Substring(startLine, endLine - startLine); startWord = startLine = endLine = ++endWord; break; case ' ': endLine = startWord = ++endWord; break; default: endWord++; break; } } }
This is a console logging function. I'm trying to get the lines to split if they don't fit the display (checked with MessageFits, included). However, the code takes a good second to execute with a message of 1000 characters or more. Is there any way this could be faster?