Changing Excel configuration

Question

This contains some complex logic and it's very difficult to provide all the things here. My intention is to simplify the loops, variable naming, and other stuff. I am thinking about splitting this big method to two or three subsidiary methods for better understanding/reading. Any ideas about that?

public static void ChangeExcelConfig(MultiQueryResultSet resultset, string[] columns)
{
 MultiQuery.TableConfig tableConfig = resultset.Results[0].TableConfig;
 int ExcelCount = tableConfig.display_config.header_rows_excel.Count();
 int locked_columns = tableConfig.display_config.datatables_numlockedcolumns;
 int columns_count = tableConfig.columns.Count;
 List<string> column_nameslist = new List<string>();
 bool is_excel = false;
 foreach (var headerrow in tableConfig.display_config.header_rows_excel)
 {
 is_excel = headerrow.header_cells[0].display_text.ToLower() == "region" ? true : is_excel;
 }
 int ExcelRegion = is_excel ? 4 : 0;
 if (columns != null && columns.Count() > 0)
 {
 int number;
 bool isNumeric = int.TryParse(columns[0], out number);
 List<MultiQuery.ColumnConfig> tableConfig_columns = new List<MultiQuery.ColumnConfig>();
 for (int j = 0; j < tableConfig.columns.Count; j++)
 {
 MultiQuery.ColumnConfig c = tableConfig.columns[j];
 if (c.display_config.enabled && c.display_config.enabled_web && c.display_config.enabled_excel)
 {
 tableConfig_columns.Add(c);
 }
 }
 if (isNumeric)
 {
 /* Getting Column names from indexes for favorite part */
 int k = locked_columns;
 /* This below section code similar to _ModelColumnChooser.cshtml (column chooser popup)*/
 for (int j = 0; j < columns_count; j++)
 {
 MultiQuery.ColumnConfig c = tableConfig.columns[j];
 if (c.column_name.StartsWith("Excel") && !c.display_config.enabled_web)
 {
 k = j + locked_columns + 1;
 }
 }
 for (int i = k; i < columns_count; i++)
 {
 MultiQuery.ColumnConfig c = tableConfig.columns[i];
 if (c.display_config.enabled && c.display_config.enabled_web)
 {
 string name = string.Join(" ", c.display_config.display_name.Distinct().ToArray());
 string column_name = Regex.Replace(name, "<.*?>", String.Empty).Replace(" ", String.Empty);
 if (!string.IsNullOrEmpty(column_name.Trim()))
 {
 column_nameslist.Add(column_name);
 }
 }
 }
 /* end column chooser section*/
 for (int i = 0; i < columns.Length; i++)
 {
 int index;
 bool bNumber = int.TryParse(columns[i], out index);
 int column_index = index - locked_columns;
 if (column_index < column_nameslist.Count)
 {
 columns[i] = column_nameslist[index - locked_columns];
 }
 }
 /* end */
 }
 int count = 0;
 for (int i = 0; i < tableConfig_columns.Count; i++)
 {
 MultiQuery.ColumnConfig c = tableConfig_columns[i];
 string name = string.Join(" ", c.display_config.display_name.Distinct().ToArray());
 string column_name = Regex.Replace(name, "<.*?>", String.Empty).Replace(" ", String.Empty);
 if (columns.Contains(column_name))
 {
 if (!c.display_config.enabled_excel) continue;
 else
 c.display_config.enabled_excel = false;
 int index = -1;
 for (int x = tableConfig.display_config.header_rows_excel.Count - 1; x > -1; x--)
 {
 bool is_removed = false;
 for (int y = tableConfig.display_config.header_rows_excel[x].header_cells.Count - 1; y > -1; y--)
 {
 //For other headers removal as well changing the columnspan
 MultiQuery.HeaderCell headerCell = tableConfig.display_config.header_rows_excel[x].header_cells[y];
 if (!String.IsNullOrEmpty(c.display_config.display_name[x]) && String.Equals(headerCell.display_text, c.display_config.display_name[x]) && x < c.display_config.display_name.Count() - 1 && y <= index)
 {
 if (!is_removed)
 {
 tableConfig.display_config.header_rows_excel[x].header_cells.Remove(headerCell);
 if (headerCell.enabled && y < tableConfig.display_config.header_rows_excel[x].header_cells.Count) tableConfig.display_config.header_rows_excel[x].header_cells[y].enabled = true;
 }
 is_removed = true;
 headerCell.colspan = headerCell.colspan > 1 ? headerCell.colspan - 1 : 1;
 }
 }
 //For Bottom Header Removal
 if (x == c.display_config.display_name.Count() - 1)
 {
 index = (i + ExcelRegion) - count;
 tableConfig.display_config.header_rows_excel[x].header_cells.RemoveAt(index); // ExcelRegion- Denotes Region,Area,District,Dealer
 }
 //For Top Headers Empty Strings Removal
 if (String.IsNullOrEmpty(c.display_config.display_name[x]) && index != -1)
 {
 MultiQuery.HeaderRow hr = tableConfig.display_config.header_rows_excel[x];
 for (int z = index; z >= 1; z--)
 {
 MultiQuery.HeaderCell hc = tableConfig.display_config.header_rows_excel[x].header_cells[z - 1];
 if (hc.colspan > 1)
 {
 hc.colspan = hc.colspan - 1;
 continue;
 }
 else if (hc.colspan == 1)
 break;
 }
 hr.header_cells.RemoveAt(index);
 }
 }
 count++;
 }
 }
 }
}

Answer 1

Some quick remarks:

• ExcelCount, ExcelRegion: should be CamelCase.
• locked_columns, columns_count, column_nameslist, is_excel,...: DO NOT use underscores, hyphens, or any other nonalphanumeric characters. MultiQueryResultSet also looks to be suffering from this significantly and it makes your code hard to read, since it doesn't look like "normal" C# code.
• .ToLower() == "region": avoid these kind of comparisons, use String.Equals.
• string[] columns: would be better if this was an ICollection<string>.
• Avoid meaningless variable names like c, k.
• Why do you start your for-loops with j, e.g. for (int j = 0;? The convention is to start with i, then use j for nested loops, etc.

---

Avoid long if-blocks:

if (columns != null && columns.Count() > 0)
{
 // 120+ lines!
}

Considering that there is no else and no other code to run, simply do:

if (columns == null || columns.Count() = 0)
{
 return;
}

---

Can't this be rewritten?

List<MultiQuery.ColumnConfig> tableConfig_columns = new List<MultiQuery.ColumnConfig>();
for (int j = 0; j < tableConfig.columns.Count; j++)
{
 MultiQuery.ColumnConfig c = tableConfig.columns[j];
 if (c.display_config.enabled && c.display_config.enabled_web && c.display_config.enabled_excel)
 {
 tableConfig_columns.Add(c);
 }
}

Using Linq:

var tableConfig_columns = tableConfig.columns
 .Where(x => x.display_config.enabled 
 && x.display_config.enabled_web 
 && x.display_config.enabled_excel)
 .ToList();

This seems to be true for several/most of your for-loops.

---

Whenever you see this pattern and you're not using i again, chances are you can use a foreach loop.:

for (int i = 0; i < tableConfig_columns.Count; i++)
{
 MultiQuery.ColumnConfig c = tableConfig_columns[i];

---

Don't do this:

if (!c.display_config.enabled_excel) continue;
else
 c.display_config.enabled_excel = false;

First of all you mix styles (your if is on one line, your else isn't), you check for a negative, and it is recommended to use braces even for single line if/else.

if (c.display_config.enabled_excel) 
{
 c.display_config.enabled_excel = false;
}
else
{
 continue;
}

---

I'd certainly recommend to split up this giant method into easier to comprehend methods, but it's rather hard to go much deeper into this code since you haven't provided us with enough context. Moreover, MultiQueryResultSet and other classes in the MultiQuery namespace seem to be custom and since they're not included...