Author | Message | Time |
---|---|---|
Insolence | [code] public class Message { //MSG_ID LAST_MESSAGE; const string MSG_ID_GAME = "0000"; const string MSG_ID_CHAT = "0010"; const string MSG_ID_USER = "1001"; const string MSG_ID_JOIN = "1002"; const string MSG_ID_LEAVE = "1003"; const string MSG_ID_WHISPER = "1004"; const string MSG_ID_TALK = "1005"; const string MSG_ID_BROADCAST = "1006"; const string MSG_ID_CHANNEL = "1007"; const string MSG_ID_USERFLAGS = "1009"; const string MSG_ID_WHISPERSENT = "1010"; const string MSG_ID_CHANNELFULL = "1013"; const string MSG_ID_CHANNELDOESNOTEXIST = "1014"; const string MSG_ID_CHANNELRESTRICTED = "1015"; const string MSG_ID_INFO = "1018"; const string MSG_ID_ERROR = "1019"; const string MSG_ID_EMOTE = "1023"; const string MSG_ID_NAME = "2010"; public string Parse ( string msg ) { //MessageBox.Show ( msg ); string returnString = ""; foreach ( string Line in msg.Split ( new char [] { '\n' } ) ) { // Doesn't even make sense, it just gets stuck right here, I commented this part out and it displays the data back correctly if ( Char.IsDigit(Line[0]) ) { string [] Word = Line.Split ( new char [] { ' ' } ); switch ( Word[0] ) { case MSG_ID_CHANNEL: return "Joining " + Word[2]; } MessageBox.Show ( Line ); } } return returnString; } }[/code] | July 26, 2005, 9:23 AM |
Myndfyr | A couple things I would suggest -- they aren't necessarily do-or-die suggestions, but they'll probably help with debugging and stepping through. 1.) Don't create variables except for simple counting variables in loops. This part: [code]string Line in msg.Split ( new char [] { '\n' } )[/code] should be split up before your foreach: [code]string msgParts = msg.Split(new char[] { '\n' }); foreach (string Line in msgParts) { ... } [/code] 2.) To further your debugging aid, use a simple for loop rather than a foreach loop. With a simple for loop, you can see what iteration you are on and more easily see during debugging what the terminating condition is. [code] int partsLength = msgParts.Length; for (int i = 0; i < partsLength; i++) { ... } [/code] Last, as a general courtesy to other people on the forum, 1.) post this in an on-topic forum: while this has to do with your bot, it's a general or .NET programming question, not a bot programming question; and 2.) Actually write a message and don't just throw code at us. I was ready to reply in disgust until I came across your comment. Comments and code all appear the same color. | July 26, 2005, 3:10 PM |
Insolence | Oh sorry, it was 4 in the morning after literally 8 hours of trying to fix with a few games of SC inbetween :) I'll be sure to post in an on-topic forum next time. Thanks for the tips, also, but do you have any idea why it's getting stuck there? It just stands still, it doesn't hit the end of the function. | July 26, 2005, 6:55 PM |
Myndfyr | [quote author=Insolence link=topic=12334.msg122051#msg122051 date=1122404140] Oh sorry, it was 4 in the morning after literally 8 hours of trying to fix with a few games of SC inbetween :) I'll be sure to post in an on-topic forum next time. Thanks for the tips, also, but do you have any idea why it's getting stuck there? It just stands still, it doesn't hit the end of the function. [/quote] Step through in the debugger and watch the value of i if you change the foreach loop to the for loop I suggested. It may never be reaching a terminating condition. | July 26, 2005, 7:51 PM |
shout | [quote author=MyndFyre link=topic=12334.msg122058#msg122058 date=1122407483] It may never be reaching a terminating condition. [/quote] In a foreach loop, that would mean that the Line variable would be an infititly large string. It should take almost no time to interate through that, even if it is hundreds of thousands of characters long. @Insolence: Bad boy! Are you using a string as a buffer? This section of code makes me think so: [code] case MSG_ID_CHANNEL: return "Joining " + Word[2]; [/code] Also, the [code] string msgParts = msg.Split(new char[] { '\n' }); [/code] could be written much simpler as: [code] string msgParts = msg.Split(0); [/code] Or the 0 can be 0x00, '\n', or even Environment.Newline. Edit: Where is the break in that switch statement? I did not know that could even compile. | July 26, 2005, 10:22 PM |
Myndfyr | [quote author=Shout link=topic=12334.msg122063#msg122063 date=1122416541] [quote author=MyndFyre link=topic=12334.msg122058#msg122058 date=1122407483] It may never be reaching a terminating condition. [/quote] In a foreach loop, that would mean that the Line variable would be an infititly large string. It should take almost no time to interate through that, even if it is hundreds of thousands of characters long. [/quote] I agree, but at the same time, I've got no idea what it could be. The code looks fine. I say, change it up a little and step through -- maybe we'll get to see what's up with the little bugger. [quote author=Shout link=topic=12334.msg122063#msg122063 date=1122416541] @Insolence: Bad boy! Are you using a string as a buffer? This section of code makes me think so: [code] case MSG_ID_CHANNEL: return "Joining " + Word[2]; [/code] [/quote] I don't know why you think he's using a string as a buffer, but he *is* using a CHAT protocol client. There's no binary about it -- all the data is in string form. You can't use a string as a buffer in C# with sockets. [quote author=Shout link=topic=12334.msg122063#msg122063 date=1122416541] Also, the [code] string msgParts = msg.Split(new char[] { '\n' }); [/code] could be written much simpler as: [code] string msgParts = msg.Split(0); [/code] Or the 0 can be 0x00, '\n', or even Environment.Newline. [/quote] Except that's not the same. The escape sequence for 0x00 is '\0', not '\n'. And you can't use Environment.NewLine there, because the Split() method takes a char[] or a params char[], and Environment.NewLine is a string (CRLF). '\n' is a newline, not NULL. [quote author=Shout link=topic=12334.msg122063#msg122063 date=1122416541] Edit: Where is the break in that switch statement? I did not know that could even compile. [/quote] The C# compiler requires that you have a terminating condition in switch cases, not a break statement -- to prevent fall-through. If he was to code: [code] switch ( Word[0] ) { case MSG_ID_CHANNEL: return "Joining " + Word[2]; break; } [/code] The C# compiler would highlight the break (well, in the IDE) and emit the warning "Unreachable code detected." This requirement of the compiler is designed to prevent the following type of code from being compiled: [code] switch (operator) { case '+': value = a + b; case '-': value = a - b; break; } return value; [/code] Note that the '+' case falls-through to the '-' case. If the compiler didn't catch this error, it would cause a runtime error that might otherwise be difficult to pin down. However, most branch instructions are legal to get out of the switch case -- break, return, continue when the switch statement is nested inside a loop, and I believe even goto is legal. | July 26, 2005, 10:45 PM |
Insolence | Man, I love this forum. MyndFyre, you're especially insightful. However, I've never used a debugger, but I'll try the For loop thing tomorrow. Having friends over today. | July 27, 2005, 12:44 AM |
Kp | [quote author=MyndFyre link=topic=12334.msg122069#msg122069 date=1122417907]This requirement of the compiler is designed to prevent the following type of code from being compiled:[code]switch (operator) { case '+': value = a + b; case '-': value = a - b; break; } return value;[/code]Note that the '+' case falls-through to the '-' case. If the compiler didn't catch this error, it would cause a runtime error that might otherwise be difficult to pin down.[/quote] From your comments, am I correct in concluding that the quoted piece of code is rejected by the C# compiler, despite that fallthrough is a classic and extremely useful feature of C? I've seen (and written!) many programs which exploit fallthrough to avoid code duplication in switch statements. Fallthrough is even used in Linux kernel code. :) | July 27, 2005, 2:01 AM |
K | I could have sworn fallthrough was allowed, but it appears you're right: [code] :~$ cat test.cs using System; namespace MyApp { public class TestApp { public static void Main(string[] args) { switch(args[1]) { case "hello": Console.WriteLine("Why hello!"); case "hi": Console.WriteLine("How are you doing?"); break; case "bye": break; } } }; }; :~$ mcs test.cs test.cs(9) error CS0163: Control cannot fall through from one case label to another Compilation failed: 1 error(s), 0 warnings [/code] However, it appears fall through is allowed if there is no specific code for each case: [code] // allowed switch(foo) { case a: case b: break; } [/code] | July 27, 2005, 2:12 AM |
Myndfyr | [quote author=Kp link=topic=12334.msg122079#msg122079 date=1122429717] From your comments, am I correct in concluding that the quoted piece of code is rejected by the C# compiler, despite that fallthrough is a classic and extremely useful feature of C? I've seen (and written!) many programs which exploit fallthrough to avoid code duplication in switch statements. Fallthrough is even used in Linux kernel code. :) [/quote] Correct. Code duplication could be avoided via refactoring, a somewhat more safe procedure. [quote author=K link=topic=12334.msg122083#msg122083 date=1122430344] However, it appears fall through is allowed if there is no specific code for each case: [/quote] Again, correct. I thought I'd mentioned that; I guess not. | July 27, 2005, 2:29 AM |
shout | Sorry, just worked a 12 hour shift and I was a little confused :( | July 27, 2005, 2:50 AM |
Kp | [quote author=MyndFyre link=topic=12334.msg122086#msg122086 date=1122431396]Correct. Code duplication could be avoided via refactoring, a somewhat more safe procedure.[/quote] I'm not familiar with the term "refactoring." How would you refactor the following code (taken from the Linux kernel)? [code] switch (how) { case 1: if (test_bit(SOCK_ASYNC_WAITDATA, &sock->flags)) break; goto call_kill; case 2: if (!test_and_clear_bit(SOCK_ASYNC_NOSPACE, &sock->flags)) break; /* fall through */ case 0: call_kill: __kill_fasync(sock->fasync_list, SIGIO, band); break; case 3: __kill_fasync(sock->fasync_list, SIGURG, band); }[/code]In particular, I'm hoping to see a solution which does not increase the number of function calls emitted. Also, are you ever going to rebut my example of what a real shell ought to do? :) If cmd can even come close to the power of tcsh, I'd definitely like to hear about it. | July 28, 2005, 2:49 AM |
Insolence | By the way, for future reference the problem was Line[0] being out of bounds, and the error occuring in a seperate thread. A simple try/catch found it out for me :D | July 31, 2005, 12:22 AM |