7
\$\begingroup\$

How can I write this more pretty?

private void _detailSpread_OnCellFocusChange(object sender, CellFocusChangeEventArgs e)
{
    if (e.SourceCol == _detailSpread.GetColNumber("article"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("account"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_1"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_2"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_3"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_4"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_5"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_6"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);
    else if (e.SourceCol == _detailSpread.GetColNumber("dim_7"))
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), _detailSpread.Rows[e.SourceRow]);

    _detailSpread.Validate();
}
\$\endgroup\$
15
\$\begingroup\$

A shorter version would be to create an anonymous array in the if statement combined with a collection initializer where you will add all of the column names and call the extension method .Any to check if any item matches the predicate:

if (new[] {"article", "account", "dim_1", ...}.Any(item => e.SourceCol == _detailSpread.GetColNumber(item)))
{
    UpdateDimensionData(_detailSpread.GetColID(e.SourceCol),
        _detailSpread.Rows[e.SourceRow]);
}
\$\endgroup\$
22
\$\begingroup\$

I don't do C# so pardon the syntax, but you should be able to reduce the amount of code by using a for loop over a string array with your column names.

Again I don't do C# but in Java you could do something like this:

for(String col : new String[]{"article", "account", "dim_1", ...}) {
    if (e.SourceCol == _detailSpread.GetColNumber(col)){
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), 
                            _detailSpread.Rows[e.SourceRow]);
        break;
    }
}

I would consider extracting the string array to a constant.

Or if your table is static you could just use a HashSet (or whatever it's called in C#):

// Somewhere in a constructor or static initializer
HashSet<Integer> focusChangeColNumbers = new ...;
colNumbers.put(_detailSpread.GetColNumber("article");
colNumbers.put(_detailSpread.GetColNumber("account");
...


// Then do:

private void _detailSpread_OnCellFocusChange(object sender, CellFocusChangeEventArgs e)
{
    if(focusChangeColNumbers.contains(e.SourceCol)){
        UpdateDimensionData(_detailSpread.GetColID(e.SourceCol), 
                            _detailSpread.Rows[e.SourceRow]);
    _detailSpread.Validate();
}

Note that this is going to be \$O(1)\$ in the number of columns where the other options are \$O(n)\$ in the number of columns.

Another option (if the API permits, I have no idea) is to simply turn it around:

String[] focusColNumbers = new String[]{"article", "account", "dim_1", ...};
if(focusColNumbers.contains(_detailSpread.GetColName(e.SourceCol))){
    ...
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.