I like that you have an actual method, instead of a bunch of code in Main. This makes is easier to reuse, segment changes, and test.
Passing in a string for horizontalDirection and verticalDirection is error prone. The caller could pass in the wrong value without realizing it. It also makes it harder to make changes with refactoring tools.
A better data type would be an enum. An example would be
enum HorizontalDirection
{
LeftToRight,
RightToLeft
}
and similar for the vertical direction.
In terms of making it simpler, the main thing that sticks out is you have essentially 4 nearly identical loops. Let's start by comparing the adding of rows. Between the LR and RL versions, the only difference is the direction the column j loop goes. The direction you go when setting all of the cell values doesn't matter in this case (because you are looking up the column with the same index you are setting in the value), so let's move the row adding loops outside the column adding loops.
Now our method looks like this:
internal DataTable GetContainerLocationMap(int row, int column, string bayCode, HorizontalDirection horizontalDirection, VerticalDirection verticalDirection)
{
var pageMap = new DataTable();
if (horizontalDirection == HorizontalDirection.LeftToRight)
{
for (int i = 1; i <= column; i++)
{
pageMap.Columns.Add("C" + i);
}
}
else if (horizontalDirection == HorizontalDirection.RightToLeft)
{
for (int i = column; i > 0; --i)
{
pageMap.Columns.Add("C" + i);
}
}
if (verticalDirection == VerticalDirection.BottomToTop)
{
for (int i = row; i > 0; --i)
{
var _pageMapRow = pageMap.NewRow();
for (int j = column; j > 0; --j)
{
_pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
}
pageMap.Rows.Add(_pageMapRow);
}
}
else if (verticalDirection == VerticalDirection.TopToBottom)
{
for (int i = 1; i <= row; i++)
{
var _pageMapRow = pageMap.NewRow();
for (int j = column; j > 0; --j)
{
_pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
}
pageMap.Rows.Add(_pageMapRow);
}
}
return pageMap;
}
Better, but there still seems to be some duplication. Let's try to condense the row adding loops.
Comapring BT against TB, the only difference is the start, end, and direction of the i loop. That means the inside of the loop is identical. Sound like a good place for another method?
internal DataTable GetContainerLocationMap(int row, int column, string bayCode, HorizontalDirection horizontalDirection, VerticalDirection verticalDirection)
{
var pageMap = new DataTable();
if (horizontalDirection == HorizontalDirection.LeftToRight)
{
for (int i = 1; i <= column; i++)
{
pageMap.Columns.Add("C" + i);
}
}
else if (horizontalDirection == HorizontalDirection.RightToLeft)
{
for (int i = column; i > 0; --i)
{
pageMap.Columns.Add("C" + i);
}
}
if (verticalDirection == VerticalDirection.BottomToTop)
{
for (int i = row; i > 0; --i)
{
AddRowToContainerMap(i, column, bayCode, pageMap);
}
}
else if (verticalDirection == VerticalDirection.TopToBottom)
{
for (int i = 1; i <= row; i++)
{
AddRowToContainerMap(i, column, bayCode, pageMap);
}
}
return pageMap;
}
private static void AddRowToContainerMap(int currentRow, int column, string bayCode, DataTable pageMap)
{
var _pageMapRow = pageMap.NewRow();
for (int j = column; j > 0; --j)
{
_pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, currentRow);
}
pageMap.Rows.Add(_pageMapRow);
}
You could get really fancy and create variables for your loop index start, end, and increment conditions. I don't think it would help readability, but this is another option for condensing code:
int iStart;
int iEnd;
int iIncrement;
if (horizontalDirection == HorizontalDirection.LeftToRight)
{
iStart = 1;
iEnd = column + 1;
iIncrement = 1;
}
else
{
iStart = column;
iEnd = 0;
iIncrement = -1;
}
for (int i = iStart; i != iEnd; i += iIncrement)
{
pageMap.Columns.Add("C" + i);
}
Comparing the readability of the two, I don't like this change.
At this point, you have two separate if blocks, each with their own for loop, and only one function inside. That's about as "simple" as I would try to make it.
Some style suggestions (the first two are the only ones I feel strongly about, the rest are very subjective):
Using an underscore in a variable name is something I expect for a member variable (field), and not a a local variable. I'd rename _pageMapRow to pageMapRow.
The variables row and column are somewhat ambiguous. At a first glance, I expected them to be an index. Turns out they are the total number of rows and columns to generate. Something like maxRow or totalRows would be clearer in my opinion.
Since we don't have any background to what your function is for, I don't know what bayCode means. If it is clear based on your context, leave the variable name as is. I presume it has to do with an identifier in a warehouse floor or similar.
C# 6 introduced string interpolation as an alternative to string.Format(). It's not necessarily more or less readable in this particular instance, but I like to share it as an option for other scenarios. If you had a longer string it may be useful to see the values as you read it. It also means you don't have to remember to change all of your indices as you add/remove parameters to string.Format. You could set the cell values using this sytax:
pageMapRow["C" + j] = $"{bayCode}-{j}-{currentRow}";
internal is an unexpected access modifier. That means your method is available for any caller inside your assembly, but not outside. It certainly has its uses, but I tend to learn toward the broader public vs private.
Final look putting all of it together
public enum HorizontalDirection
{
LeftToRight,
RightToLeft
}
public enum VerticalDirection
{
TopToBottom,
BottomToTop
}
public DataTable GetContainerLocationMap(int totalRows, int totalColumns, string bayCode, HorizontalDirection horizontalDirection, VerticalDirection verticalDirection)
{
var pageMap = new DataTable();
if (horizontalDirection == HorizontalDirection.LeftToRight)
{
for (int i = 1; i <= totalColumns; i++)
{
pageMap.Columns.Add("C" + i);
}
}
else if (horizontalDirection == HorizontalDirection.RightToLeft)
{
for (int i = totalColumns; i > 0; --i)
{
pageMap.Columns.Add("C" + i);
}
}
if (verticalDirection == VerticalDirection.BottomToTop)
{
for (int i = totalRows; i > 0; --i)
{
AddRowToContainerMap(i, totalColumns, bayCode, pageMap);
}
}
else if (verticalDirection == VerticalDirection.TopToBottom)
{
for (int i = 1; i <= totalRows; i++)
{
AddRowToContainerMap(i, totalColumns, bayCode, pageMap);
}
}
return pageMap;
}
private static void AddRowToContainerMap(int currentRow, int column, string bayCode, DataTable pageMap)
{
var pageMapRow = pageMap.NewRow();
for (int j = column; j > 0; --j)
{
pageMapRow["C" + j] = $"{bayCode}-{j}-{currentRow}";
}
pageMap.Rows.Add(pageMapRow);
}
