current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
up vote 5 down vote favorite

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();
}
share

locked by janos♦ 1 hour ago

This post has been locked while disputes about its content are being resolved. For more info visit meta.
6  
switch? But aside from that, all your actions are the same, no matter which branch is taken. – insertusernamehere 18 hours ago
4  
Maybe it's shorter to test against a blacklist? Like if (!e.SourceCol == _detailSpread.GetColNumber("abc") { UpdateDimensionData(); }? It would be interesting to see, what other values are possible. – insertusernamehere 18 hours ago
2  
@JanErikGunnar thats not really the point of the question above. The principle should be applicable to any if-syntax with many conditions... – dadde 17 hours ago
2  
The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. – Jamal♦ 10 hours ago
2  
The desire to improve code is implied for all questions on this site. Question titles should reflect the purpose of the code, not how you wish to have it reworked. – Jamal♦ 5 hours ago
up vote 11 down vote accepted

Expanding on Emily L. answer.

You can do:

if (new[] {"article", "account", "dim_1", ...}.Any(item => e.SourceCol == _detailSpread.GetColNumber(item)))
{
    UpdateDimensionData(_detailSpread.GetColID(e.SourceCol),
        _detailSpread.Rows[e.SourceRow]);
}
share
    
thank you, this looks nicer, and also thanks to Emily L. – dadde 16 hours ago
up vote 17 down vote

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))){
    ...
}
share

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