▲ 7 ▼ Help with a complex bug (github.com) submitted 4 months ago by dr_robotBones@reddthat.com to c/csharp@programming.dev 4 comments fedilink hide all child comments
[+] dr_robotBones@reddthat.com [S] 2 points 4 months ago (2 children) [deleted] permalink fedilink source parent hideshow 2 child comments replies: [–] xianjam@programming.dev 3 points 4 months ago* (1 child) Stack doesn't actually allow indexing, and ElementAt is an extension method implemented by accessing Stack's iterator. My honest recommendation is to not use Stack, even if that's what the book was using. Stacks are not really meant to be traversed. They're meant to be pushed to, and popped (or peeked) from. Each ElementAt call requires O(N) time to retrieve. I recommend switching to List and implementing 'push', 'pop', and 'peek' as such: // Instead of Push scopes.Add(item); // Instead of Peek int peeked = scopes[^1]; // Instead of Pop int popped = scopes[^1]; scopes.RemoveAt(scopes.Count - 1); However, I understand that is a bit of a change. If you'd prefer to keep it as a stack, a simple reversal of the for loop can get the job done. // Resolve local variable assignment private void ResolveLocal(Expr expr, Token name) { // Decrement through scopes for (int i = 0; i < scopes.Count; i++) { // Pass amount of layers since declaration to interpreter if (scopes.ElementAt(i).ContainsKey(name.getLexeme())) { interpreter.Resolve(expr, i); return; } } } That's enough to get these scripts running. >for(var i = 0; i < 10; i = i + 1) { print i;} 0 1 2 3 4 5 6 7 8 9 >var a = 0; var temp; for(var b= 1; a < 10000; b = temp + b) { print a; temp = a; a = b;} 0 1 1 2 3 5 8 13 21 34 55 89 144 233 377 610 987 1597 2584 4181 6765 By the way, I noticed that redefining a global variable will crash as well, but a scoped variable will display an error as intended. permalink fedilink source parent hideshow 1 child comment load more comments (1 reply)
[–] xianjam@programming.dev 3 points 4 months ago* (1 child) Stack doesn't actually allow indexing, and ElementAt is an extension method implemented by accessing Stack's iterator. My honest recommendation is to not use Stack, even if that's what the book was using. Stacks are not really meant to be traversed. They're meant to be pushed to, and popped (or peeked) from. Each ElementAt call requires O(N) time to retrieve. I recommend switching to List and implementing 'push', 'pop', and 'peek' as such: // Instead of Push scopes.Add(item); // Instead of Peek int peeked = scopes[^1]; // Instead of Pop int popped = scopes[^1]; scopes.RemoveAt(scopes.Count - 1); However, I understand that is a bit of a change. If you'd prefer to keep it as a stack, a simple reversal of the for loop can get the job done. // Resolve local variable assignment private void ResolveLocal(Expr expr, Token name) { // Decrement through scopes for (int i = 0; i < scopes.Count; i++) { // Pass amount of layers since declaration to interpreter if (scopes.ElementAt(i).ContainsKey(name.getLexeme())) { interpreter.Resolve(expr, i); return; } } } That's enough to get these scripts running. >for(var i = 0; i < 10; i = i + 1) { print i;} 0 1 2 3 4 5 6 7 8 9 >var a = 0; var temp; for(var b= 1; a < 10000; b = temp + b) { print a; temp = a; a = b;} 0 1 1 2 3 5 8 13 21 34 55 89 144 233 377 610 987 1597 2584 4181 6765 By the way, I noticed that redefining a global variable will crash as well, but a scoped variable will display an error as intended. permalink fedilink source parent hideshow 1 child comment load more comments (1 reply)