Author Topic: Highlight consumers and/or suppliers of a factory  (Read 245 times)

0 Members and 1 Guest are viewing this topic.

Offline hreintke

Highlight consumers and/or suppliers of a factory
« on: August 19, 2017, 03:52:00 PM »
I always have trouble to find/visualize the consumers and suppliers of  a factory.

This patch adds two buttons to the factory_info window.
These are "toggle buttons" which, when pressed, highlight either the consumers or the suppliers of the factory.

The text in the supplier button has an additional ":".
I do not know why that is displayed, needs to be removed.

attached the diff for the patch, based on simutrans trunk

Comments welcome



Offline DrSuperGood

Re: Highlight consumers and/or suppliers of a factory
« Reply #1 on: August 19, 2017, 05:34:54 PM »
Looks like there quite a bit of procedural coupling, similar statements are repeated with slightly different arguments. This is generally bad for maintainability and should be avoided when possible.

Offline hreintke

Re: Highlight consumers and/or suppliers of a factory
« Reply #2 on: August 20, 2017, 07:22:47 AM »
No problem if it is not going to be incorporated, I will just use it as one of my local updates.
But I do not understand your statement on procedural coupling. Can you explain what you mean ?

Offline DrSuperGood

Re: Highlight consumers and/or suppliers of a factory
« Reply #3 on: August 20, 2017, 04:32:43 PM »
Quote
Can you explain what you mean ?
I thought I did...
Quote
similar statements are repeated with slightly different arguments
It is generally bad for coupling to repeat the same procedure implementation multiple times as if that has to be changed for whatever reason there are then multiple copies that have to be changed not only making it take longer to make the change, but also making it more error prone as one copy might not be modified correctly.

For example this code has bad procedural coupling.
Code: [Select]
thing_t A, B;
DoSomething(A);
DoSomethingElse(A);
if( SomeTest(A) ){
    DoAnotherThing(A);
}
DoSomething(B);
DoSomethingElse(B);
if( SomeTest(B) ){
    DoAnotherThing(B);
}
The same procedure code is repeated twice with the only difference being what thing_t it is applied to. If one wanted to change the procedure, say by adding an extra function call to DoAwesomeThing(thing_t) then one would have to add the function call in both instances of the procedure. The procedural coupling could be reduced like this...
Code: [Select]
void DoStuff(thing_t thing) {
    DoSomething(thing);
    DoSomethingElse(thing);
    if( SomeTest(thing) ){
        DoAnotherThing(thing);
    }
}
...
thing_t A, B;
DoStuff(A);
DoStuff(B);
After this change one could change all uses of the procedure by just modifying the DoStuff function, so adding a call to DoAwesomeThing(thing_t) is then trivial.

You can see such coupling in your changes to some extent.
Code: [Select]
+ if (fab->get_lieferziele().get_count() > 0){
+ highlight_consumer_button.init(button_t::roundbox_state, "Consumers", scr_coord(BUTTON1_X, offset_below_viewport));
+ highlight_consumer_button.set_tooltip("Highlight consumers");
+ highlight_consumer_button.add_listener(this);
+ add_component(&highlight_consumer_button);
+ }
+
+ if (fab->get_suppliers().get_count() > 0){
+ highlight_supplier_button.init(button_t::roundbox_state, "Suppliers", scr_coord(BUTTON2_X, offset_below_viewport));
+ highlight_supplier_button.set_tooltip("Highlight suppliers");
+ highlight_supplier_button.add_listener(this);
+ add_component(&highlight_supplier_button);
+ }