euphorie / Euphorie

Euphorie is a tool for risk assessment
https://pythonhosted.org/Euphorie/
GNU General Public License v2.0
12 stars 6 forks source link

Show solution descriptions in the Word report #596

Closed reinhardt closed 1 year ago

reinhardt commented 1 year ago

except for France and Italy when measures in place are active

syslabcom/scrum#1245

reinhardt commented 1 year ago

Thanks for the suggestions. I see even more potential to improve this code. I'll check how much effort we can spend.

reinhardt commented 1 year ago

Generally speaking, I think it is a good practice to not introduce feature flags and if all over the place to handle custom behaviors because it often leads to code which is harder to maintain.

What would you say is a better alternative?

ale-rt commented 1 year ago

Generally speaking, I think it is a good practice to not introduce feature flags and if all over the place to handle custom behaviors because it often leads to code which is harder to maintain.

What would you say is a better alternative?

See my suggestion in https://github.com/euphorie/Euphorie/pull/596#discussion_r1221339895

You code looks like


class A:
    flag_active = False

    def __call__(self):
        if self.flag_active:
            return "something"
        else:
            return "something else"

class B(A):
    flag_active = True

My suggestion is to make it look like to this:


class A:
    def __call__(self):
        return "something"

class B(A):
    def __call__(self):
        return "something else"

That's especially helpful in this case because of the cyclomatic complexity of the method.

reinhardt commented 1 year ago

Oh, that's what you mean. Now I get it, thanks. I do see the benefits. On the other hand, if I have a flag with a speaking name like use_solution_descriptions, it is potentially clearer what the intention is.

reinhardt commented 1 year ago

I'll merge this now and clean up in a new pull request.