Code quality: Single source

[Spike]

Duplicate code means that bugs must be fixed in both occurences of the code fragment. If a programmer forgets (or doesn't know) the other places where a code fragment exists, they can only fix the bug in one place, while "single source" would have it fixed for all usages.

In citylist frame class, there is this code:

Code Select Expand


    sort_label.set_pos(koord(BUTTON1_X, 40-BUTTON_HEIGHT-(LINESPACE+1)));
    add_komponente(&sort_label);

    sortedby.init(button_t::roundbox, "", koord(BUTTON1_X, 40-BUTTON_HEIGHT), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sortedby.add_listener(this);
    add_komponente(&sortedby);

    sorteddir.init(button_t::roundbox, "", koord(BUTTON2_X, 40-BUTTON_HEIGHT), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sorteddir.add_listener(this);
    add_komponente(&sorteddir);



In curiousity list frame class:

Code Select Expand


    sort_label.set_pos(koord(BUTTON1_X, 2));
    add_komponente(&sort_label);

    sortedby.init(button_t::roundbox, "", koord(BUTTON1_X, 14), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sortedby.add_listener(this);
    add_komponente(&sortedby);

    sorteddir.init(button_t::roundbox, "", koord(BUTTON2_X, 14), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sorteddir.add_listener(this);
    add_komponente(&sorteddir);


In factory list frame class there is this:

Code Select Expand


    sort_label.set_pos(koord(BUTTON1_X, 2));
    add_komponente(&sort_label);

    sortedby.init(button_t::roundbox, "", koord(BUTTON1_X, 14), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sortedby.add_listener(this);
    add_komponente(&sortedby);

    sorteddir.init(button_t::roundbox, "", koord(BUTTON2_X, 14), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sorteddir.add_listener(this);
    add_komponente(&sorteddir);

    // name buttons
    sortedby.set_text(sort_text[get_sortierung()]);
    sorteddir.set_text(get_reverse() ? "hl_btn_sort_desc" : "hl_btn_sort_asc");


In halt list frame there is this:

Code Select Expand


    sort_label.set_pos(koord(BUTTON1_X, 2));
    add_komponente(&sort_label);
   
    sortedby.init(button_t::roundbox, "", koord(BUTTON1_X, 14), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sortedby.add_listener(this);
    add_komponente(&sortedby);

    sorteddir.init(button_t::roundbox, "", koord(BUTTON2_X, 14), koord(BUTTON_WIDTH,BUTTON_HEIGHT));
    sorteddir.add_listener(this);
    add_komponente(&sorteddir);

    filter_label.set_pos(koord(BUTTON3_X, 2));
    add_komponente(&filter_label);



This code is not 100% identical, but could be merged into one method with a few parameters. These list frames could have a common base class, which contains the common code, and the special features of each list frame go into the subclasses.

There are more, but in some of the UI classes it's most obvious.

[kierongreen]

Agreed - I've noticed this when playing about with code recently too.

[isidoro]

I agree too.  This is a very basic aspect of programming.

[el_slapper]

I agree too.  This is a very basic aspect of programming.

True, but it does not mean it's easy to always keep clean of duplicate code. Some modularization sometimes appear obvious loooong after the duplication has been done.

[prissi]

The original code stemmed actually from a signle person, thus those listing (and the never was done vehicle list) could very well be based on a list dialog class. Extra filter buttons would be then by deriving from this base class.

I thing one would also not implement and action listener and the sort criteria would be rather a combobox nowadays ...