The International Simutrans Forum

 

Author Topic: Code quality: Single source  (Read 1363 times)

0 Members and 1 Guest are viewing this topic.

Offline Spike

  • *
  • Posts: 1361
  • First Simutrans Developer and Graphics Artist
Code quality: Single source
« on: February 27, 2012, 03:03:31 PM »
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]
    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]
    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]
    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]
    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.


Offline kierongreen

  • Dev Team, Coder/patcher
  • Devotee
  • *
  • Posts: 2269
Re: Code quality: Single source
« Reply #1 on: February 27, 2012, 04:33:57 PM »
Agreed - I've noticed this when playing about with code recently too.

Offline isidoro

  • Devotee
  • *
  • Posts: 1129
Re: Code quality: Single source
« Reply #2 on: February 27, 2012, 10:34:45 PM »
I agree too.  This is a very basic aspect of programming.

Offline el_slapper

  • *
  • Posts: 211
  • Languages: FR, EN, DE
Re: Code quality: Single source
« Reply #3 on: February 28, 2012, 10:53:01 AM »
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.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9516
  • Languages: De,EN,JP
Re: Code quality: Single source
« Reply #4 on: February 28, 2012, 11:03:19 AM »
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 ...