0
namespace Random_walk_2d
{
    internal class Program
    {
        static void Main(string[] args)
        {
            Point C = new Point(2, 5);

            C.Move('n');
            Point.Print(C);
        }
    }

    public class Point
    {
        public int x;
        public int y;

        readonly public Dictionary<char, Point> directions = new Dictionary<char, Point>()
        {
            { 'e', new Point(1, 0) },
            { 'n', new Point(0, 1) },
            { 'w', new Point(-1, 0) },
            { 's', new Point(0, -1) },
        };

        public Point(int x, int y)
        {
            this.x = x;
            this.y = y;
        }

        public static Point operator +(Point A, Point B)
        {
            return new Point(A.x + B.x, A.y + B.y);
        }

        public static void Print(Point A)
        {
            Console.WriteLine($"({A.x}, {A.y})");
        }

        public void Move(char dir)
        {
            Point displacement = directions[dir];
            this.x += displacement.x;
            this.y += displacement.y;
        }

    }
}

I can see that the problem lies somewhere within the dictionary or the Move() method, but what I don't understand is what's actually going on under the hood? What's causing the infinite loop to occur?

And how can I solve the issue?

My friend suggested me to use arrays of 2 ints instead of Points in the dictionary, but... that kinda defeats the purpose of the whole class. Why use Points if I can just use 2 int arrays? I want a solution that doesn't do that.

If you have any other kind of advice, do let me know.

EDIT: I should've mentioned this right away, but when I run it, I get an infinite amount of at Random_walk_2d.Point..ctor(Int32, Int32) messages in the console.

EDIT2: I'm just experimenting with classes. For this project I wanted to create a random walk on a 2d grid. A 2d grid can be nicely modeled by a class with 2d point objects. I need the Move() method to move 1 unit to the right(east = 'e'), up(north = 'n') and so on.

Imaginary
  • 75
  • 7
  • what makes you think its in an infinite loop – pm100 Jan 10 '23 at 00:03
  • It is, I click on "run" and get an infinite amount of `at Random_walk_2d.Point..ctor(Int32, Int32)` – Imaginary Jan 10 '23 at 00:05
  • Using arrays of two ints in the dictionary is not cheating. A class implements a concept, it does not need to also be implemented in terms of itself. But there is a better solution anyway. – harold Jan 10 '23 at 00:11

3 Answers3

3

the problem is stack overflow, this done here

public class Point {
    public int x;
    public int y;

    readonly public Dictionary<char, Point> directions = new Dictionary<char, Point>()
    {
        { 'e', new Point(1, 0) },
        { 'n', new Point(0, 1) },
        { 'w', new Point(-1, 0) },
        { 's', new Point(0, -1) },
    };

when you create a Point, that creates a 'directions' member, this in turn creates 4 new Points, each of those new Points creates its own 'directions' each one creates 4 Points .....

Its not clear what you are trying to do but this fixed it

public class Point {
    public int x;
    public int y;

    static readonly public Dictionary<char, Point> directions = new Dictionary<char, Point>()
    {
        { 'e', new Point(1, 0) },
        { 'n', new Point(0, 1) },
        { 'w', new Point(-1, 0) },
        { 's', new Point(0, -1) },
    };

By declaring 'directions' static there is only one for the whole Point class, maybe thats what you want

EDIT - yes this is what you want the 'directions' map is effectively a constant, so static works fine

pm100
  • 48,078
  • 23
  • 82
  • 145
  • THANKS that's exactly what I needed. The static keyword has been pretty confusing to me, but this solution actually makes sense. – Imaginary Jan 10 '23 at 00:24
  • 1
    @Imaginary - here `static` means "I want only one for the whole class", this is exactly what you need for a global lookup table – pm100 Jan 10 '23 at 00:34
2

You are recursively, indefinitely creating new objects of type Point, because upon creation of Point, you are creating a directions with 4 points, each of which have a dictionary with 4 more points, etc.

screenshot with recursion explanation

I'm unsure what exactly is that you're trying to achieve, because that's unclear from the question. so I can't help any further.

KifoPL
  • 981
  • 7
  • 18
  • I see why my implementation is an issue, thanks for the explanation. Btw I updated the question, check it out to see what I'm trying to achieve. – Imaginary Jan 10 '23 at 00:22
1

I recommend using static variables to hold your specific directions instead of a dictionary. I also recommend, not using classes but instead use a struct in order to ensure the data isn't changed my accident. A struct has value semantics, which means the pair of (x,y) go together always.

Instead of a dictionary, there is a function with a switch statement that returns the proper step. Then I am doing a trick and overwriting the this keyword in Move() to change the values of (x,y) in one go. If you already have the operator + defined, might as well use it.

static class Program
{
    static void Main(string[] args)
    {
        Point C = new Point(2, 5);
        C.Move('n');
        Console.WriteLine(C);
    }
}

public struct Point
{
    public readonly int x;
    public readonly int y;

    public Point(int x, int y)
    {
        this.x = x;
        this.y = y;
    }

    public static readonly Point North = new Point(0, 1);
    public static readonly Point South = new Point(0, -1);
    public static readonly Point East = new Point(1, 0);
    public static readonly Point West = new Point(-1, 0);

    public Point GetDirection(char dir)
    {
        switch (dir)
        {
            case 'n': return North;
            case 'e': return East;
            case 's': return South;
            case 'w': return West;
            default:
                throw new ArgumentException("Invalid Direction.", nameof(dir));
        }
    }

    public static Point operator +(Point a, Point b) => new Point(a.x + b.x, a.y + b.y);

    public void Move(char dir)
    {
        Point delta = GetDirection(dir);
        this += delta;
    }
    public override string ToString()
    {
        return $"( {x}, {y} )";
    }
}
John Alexiou
  • 28,472
  • 11
  • 77
  • 133
  • This is also a cool approach, and structs seem like a better object to use for this case, rather than classes. Thanks a lot for all that! – Imaginary Jan 10 '23 at 01:13