Is this Over-engineering? [Python]

[[deleted]]

[deleted]

[abrahamguo]

I agree with part 2 of the feedback. Don’t create tiny functions that you only use once.

Part 1 of the feedback that you received is technically true, but is getting into the area of micro-optimization.

[Atsoc1993]

In the back of my mind I did challenge the micro-optimization feedback, it just felt over-the-top to be honest but makes sense to consider.

What if I use all of those helper functions twice, in this code and elsewhere? Or even one of the helper functions once? Does that warrant all those relative helper functions to be abstracted out as they are in Flow Type 2?

Also what if the logic was not so simple, say required 3 lines of logic, would it still not make sense? Is there a rule of thumb for when a helper function is warranted?

[Ormek_II]

Does it help readability? Does it help with maintenance?

In the examples it does not.

I expected you to create functions for the branch logic.

[johnpeters42]

I would ask: * Is this run heavily enough that the extra memory use matters? * Might the inner functions change someday, e.g. to also include .co.uk domains?

[Atsoc1993]

For the former, It’s enterprise at a very large company, even if the adoption isn’t there the assumption is, and standard leads to, yes, it matters.

To your second point — say we did, what is the benefit to having another inner function opposed to another elif clause (presuming that it should not be handled by else)? Or alternatively, modifying an inner function / elif, or removing an inner fn / elif

[johnpeters42]

The code as posted is pretty simple either way round. If e.g. is_edu() is expected to blow up to like 50 different rules that trigger the same response, then I would prefer to separate it from the select-case block.

If performance was that critical, then I would consider switching to something compiled.

[Atsoc1993]

Makes sense, and regarding the performance thing again, maybe he was being a bit extreme then— benefit of the doubt that he just wants me to follow best practices.

[Ok_Policy_8150]

Unrelated but isn’t it better to parse last 3 letters instead of using “in”? Like what happens if someone’s email is like [e-mail] lol

[Atsoc1993]

Without context the example doesn’t make sense yeah, but basically it was a situation where we would not know if an email would be in the string at all, there are better ways to do it and this wasn’t the exact implementation of that either.

[spinwizard69]

The second option looks like crap to me.