HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

How to shorten two methods with the same theme?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
samethewithshortenmethodstwothemehow

Problem

I have this code:

private void numWidth_ValueChanged(object sender, EventArgs e)
{
    if (ignoreChange) return; // Sometimes I only want to change the value without the event firing (like when undo-ing)

    PushUndoStack();

    int width = (int)numWidth.Value;

    foreach (Layer layer in doc.TileLayers)
        layer.Width = width;

    hscDoc.Maximum = doc.WidthInPixels;
    ClampScrollbarValue(hscDoc);
}

private void numHeight_ValueChanged(object sender, EventArgs e)
{
    if (ignoreChange) return;

    PushUndoStack();

    int height = (int)numHeight.Value;

    foreach (Layer layer in doc.TileLayers)
        layer.Height = height;

    vscDoc.Maximum = doc.HeightInPixels;
    ClampScrollbarValue(vscDoc);
}


They are both very similar yet I can't really think of a good way to combine the two. This happens several times in my program and the lines of code are really adding up. I suppose I could use reflection but is it overkill? Can anyone think of a better way?

This is just an example. This happens quite a few times in my program where functions are very similar but cannot be easily refactored in to a generic method.

Solution

Something along these lines:

private void ValueChangedImp(object sender, EventArgs e, Func get_value, Action set_value, Func get_pixels)
{
    if (ignoreChange) return; // Sometimes I only want to change the value without the event firing (like when undo-ing)

    PushUndoStack();

    int value = get_value();

    foreach (Layer layer in doc.TileLayers)
        set_value(laywe, value);

    hscDoc.Maximum = get_pixels();
    ClampScrollbarValue(hscDoc);
}

private void numWidth_ValueChanged(object sender, EventArgs e)
{
    ValueChangedImp(sender, e, () => (int)numWidth.Value, (layer, value) => layer.Width = value, () => doc.WidthInPixels);
}

private void numHeight_ValueChanged(object sender, EventArgs e)
{
    ValueChangedImp(sender, e, () => (int)numHeight.Value, (layer, value) => layer.Height = value, () => doc.HeightInPixels);
}

Code Snippets

private void ValueChangedImp(object sender, EventArgs e, Func<int> get_value, Action<Layer, int> set_value, Func<int> get_pixels)
{
    if (ignoreChange) return; // Sometimes I only want to change the value without the event firing (like when undo-ing)

    PushUndoStack();

    int value = get_value();

    foreach (Layer layer in doc.TileLayers)
        set_value(laywe, value);

    hscDoc.Maximum = get_pixels();
    ClampScrollbarValue(hscDoc);
}

private void numWidth_ValueChanged(object sender, EventArgs e)
{
    ValueChangedImp(sender, e, () => (int)numWidth.Value, (layer, value) => layer.Width = value, () => doc.WidthInPixels);
}

private void numHeight_ValueChanged(object sender, EventArgs e)
{
    ValueChangedImp(sender, e, () => (int)numHeight.Value, (layer, value) => layer.Height = value, () => doc.HeightInPixels);
}

Context

StackExchange Code Review Q#4528, answer score: 5

Revisions (0)

No revisions yet.