HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Crawling 2D Maze/Matrix

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
crawlingmatrixmaze

Problem

I'm trying to create a class that's able to 'crawl' through a generic 2D matrix.
Crawling through a Bitmap (which can be viewed as a 2D Matrix of Colors) should yield the same result of a flood fill.

So the first thing I did was create a IMatrix (previous name was I2DMatrix. I renamed it since i don't intend to code a I3DMatrix) interface.

public interface IMatrix
{
    int Width { get; }
    int Height { get; }

    void SetValue(int x, int y, T value);
    void SetValue(Point pt, T value);

    T GetValue(int x, int y);
    T GetValue(Point pt);
}


Then I created a abstract crawler for a genertic type T. Abstract so I don't have to worry creating 'how do I mark this place as already visited' and 'is this point crawlable' methods.
The code for the Crawler is at the end of the post. It's somewhat polluted with comments, so here's a brief explanation of what's what.

  • Protected non-abstract methods: are responsible for actually 'crawling' through the IMatrix. They rely heavily on IsValid(Point pt) and Mark(Pt), both abstract methods. If you override them to work with images in such way that Mark(Point pt) actually changes the color of the pixel at Pt and IsValid(Point pt) returns true whenever the color at Pt should be changed and false whenever it should not, then you'd have the flood fill effect previously mentioned.



  • ValidArea property: used to create a boundary inside the IMatrix



  • If you have any questions about the code, I'll update this list.



I'd like to have some help improving the Crawl(Point pt) method performance. From what I've read, I'm doing some unnecessary checking of points (as in checking points multiple times). But I don't know how I could improve it.

```
public abstract class Crawler
{
#region Fields
///
/// The crawlable 2D grid.
///
protected IMatrix matrix;

///
/// The width of the crawlable grid.
///
protected int Width { get { return matrix.Width; } }

///
/// The hei

Solution

This is really very good in my opinion. I like your use of interfaces, and an abstract base class seems to be the right design choice to me.

The double checks here bother me a little bit though.

while (upperRows.Count > 0 || lowerRows.Count > 0)
    {
        if (upperRows.Count > 0)
        {
            CheckLocation(upperRows.Pop());
        }
        if (lowerRows.Count > 0)
        {
            CheckLocation(lowerRows.Pop());
        }
    }


Unfortunately, I don't see a way to do it that wouldn't add an extra loop and given a loop would be much more inefficient than accessing the property repeatedly, I think it's good.

Properties should be PascalCased. You have a couple that are camelCased here.

protected Stack upperRows { get; set; }

/// 
/// Used to 'jump' to a walkable position after the crawler is no longer able to move left or right, without having to backtrack.
/// 
protected Stack lowerRows { get; set; }


CheckLeft, CheckRight, etc. make sense, but this one's name could be better IMO.

/// 
/// Check if the point (pt.X, pt.Y - 1) should be walked. If it should, add it to the lowerRows stack.
/// 
/// The point whose 'bot' will be checked.
protected void CheckBot(Point pt)


The doc comment seems to be at odds with the name CheckBot. I would expect this to check on the current location of the crawler. Which, on further inspection, is exactly what it does. So, the comment is a bit misleading. It's not obvious what this method does by either its name or comment.

The doc comments are great to have by the way. Did you know that you can reference types in them?

/// Check if the  (pt.X + 1, pt.Y) should be walked through. If it should, move the crawler there and check its surroundings.


  • Recommended doc comment tags



  • see tag example

Code Snippets

while (upperRows.Count > 0 || lowerRows.Count > 0)
    {
        if (upperRows.Count > 0)
        {
            CheckLocation(upperRows.Pop());
        }
        if (lowerRows.Count > 0)
        {
            CheckLocation(lowerRows.Pop());
        }
    }
protected Stack<Point> upperRows { get; set; }

/// <summary>
/// Used to 'jump' to a walkable position after the crawler is no longer able to move left or right, without having to backtrack.
/// </summary>
protected Stack<Point> lowerRows { get; set; }
/// <summary>
/// Check if the point (pt.X, pt.Y - 1) should be walked. If it should, add it to the lowerRows stack.
/// </summary>
/// <param name="pt">The point whose 'bot' will be checked.</param>
protected void CheckBot(Point pt)
/// Check if the <see cref="Point" /> (pt.X + 1, pt.Y) should be walked through. If it should, move the crawler there and check its surroundings.

Context

StackExchange Code Review Q#92207, answer score: 2

Revisions (0)

No revisions yet.