There is no need to have
n as global variables. Create a structure to hold your data members together and pass a pointer to that structure to your functions.
Variable names are too short and not intuitive:
y etc. Use better names (ex:
numberOfCols etc) that explain what the variable is doing.
For readability reasons I would go with a recursive implementation. My second choice would be a stack based, if recursivity is not an option. And if performance would really be an important point of my implementation that I would do something similar but:
I would create a struct
MatrixMap that would hold my
I would represent the position of an element from the
MatrixMap using a structure
MatrixMapCoordinate having two
An element from the
MatrixMap can be represented using a structure
Accessing an element can be done with a function similar with:
const MatrixMapElement* ElementAt(const MatrixMap* fromMap, const MatrixMapCoordinate* atPosition);
I would move in my matrix using specialized functions such as
, MoveEast and
MoveSouth. You can even send flags to control the movement behavior:
Lastly, what I said at point
3 I would rewrite my algorithm as close as possible to the actual pseudocode.
//Read "elements" as color in the algorithm.
// I decided to use element instead of color so you can use it in any kind of problem, regardless of what does your matrix contains.
void FloodFill(MatrixMap* targetMap, const MatrixMapCoordinate* startingCoordinate, const MatrixMapElement* targetElement, const MatrixMapElement* replacementElement)
//If the starting coordinate is out of range we end the fill
if (HasElementAt(targetMap, startingCoordinate))
//Check if the element at the startingCoordinate is of the targetElements.
if (ElementsAreEqual(ElementAt(targetMap, startingCoordinate), targetElement))
//If both target and replacement are equal there is nothing to do.
if (ElementsAreEqual(targetElement, replacementElement))
SetElementValue(targetMap, startingCoordinate, replacementElement);
FloodFill(targetMap, MoveWest(startingCoordinate), targetElement, replacementElement);
FloodFill(targetMap, MoveNorth(startingCoordinate), targetElement, replacementElement);
FloodFill(targetMap, MoveSouth(startingCoordinate), targetElement, replacementElement);
FloodFill(targetMap, MoveEast(startingCoordinate), targetElement, replacementElement);
Note that there is no error or pointer check. I wrote the code just to give you an idea on how it should look from a readability point of view.
You can do a similar implementation for any algorithm you choose.
Because of the poor readability in your initial implementation I didn't spend time to validate your logic. Though I would be more than happy to do it, if you refactor your code using some of the suggestions I gave you.