Avoid to call global functions from any class method

Unknown
Unknown
edited August 2018 in Dev & Ops

Since we agreed here to not use global functions. I have a next question-proposal:

Whenever we code any class especially when we do refactoring or legacy code function move to other place (to make it not global, but for example static function of some class) we need to avoid calling global functions at all.

For example, now we have this 2 global functions:

function saveToConfig($name, $value = '', $options = []) {
    Gdn::config()->saveToConfig($name, $value, $options);
}
function c($name = false, $default = false) {
    return Gdn::config($name, $default);
}

That is obvious that we already have namespaced clones.

That means we should call Gdn::config()->saveToConfig() instead of global saveToConfig()

Some global functions will be moved to some classes. Every developer are welcome to make such transformation for any global function if they work on some code where that function is required.

As a first phase we should get all global functions namespaced analogues.

But please do not do massive conversion. Apply only on the portion you work with and need for your task to get working. Discuss with colleagues to coordinate and to not apply "conversion" of the same function to put them into different places/classes.


Main point for this post to agree :

We should avoid using global functions everywhere.

Stop using global functions!

Comments

  • Unknown
    Unknown

    @Linc @Ryan @Todd please vote :-)

  • Vanilla Forums
    Vanilla Forums

    Reducing complex situations to a yes or no vote makes me very reticent, so you're unlikely to see me casting ballots. I am tentatively onboard. I'll be keen to see how you resolve these issues where the rubber meets the road.

  • Unknown
    Unknown

    My thoughts are:

    1. Global functions that have no dependencies or side-effects can be made into static methods on utility classes. That's fine.
    2. Global functions that are providing forwards compatibility with functions that are added to the standard library can and should be implemented until our project's version requirements are moved forward enough to remove them again. This is rare, but a good example is array_column which incidentally should be removed at this point since we are way past PHP 5.4 now.
    3. Global functions that call into static objects that represent dependencies should be removed and done with dependency injection. This specifically means any Gdn::* should no longer be used. I would rather see calls to c() remain rather than have them changed to Gdn::config(...).
    4. Our specific use of saveToConfig(..., ..., false) should no longer be used. Rather, we should have anything that is set from config be a property on the object and have that property changed itself. We should be only saving to the config on administration pages and in that case injecting the config object is appropriate.

    Finally, I want to note that this is a development practice that should not hurt product requirements. This came up in our knowledge base planning meeting where we had a need for sliceParagraph() like functionality. If you don't want to use global functions anymore then you have to implement many existing functions elsewhere, not ask to remove product functionality that makes use of those global functions.

  • Unknown
    Unknown
    edited August 2018

    But something like a slice paragraph function has no reason to be a global function. That belongs in a string utility class, probably as a static method.

  • Vanilla Forums
    Vanilla Forums
    https://staff.vanillaforums.com/discussion/comment/1775#Comment_1775

    Todd and I are both saying the same thing from different angles. It's a worthy and probably attainable goal, but:

    • The devil is in the details.
    • Don't delay product goals because you can't do it in the most-optimal way immediately.
    • Don't follow your holy grail quest blindly, or you'll end up with bigger problems.

    If implementing that class is something you can do without doubling the complexity of the task, great. But we don't wanna hear "this functionality (we already have) is too difficult" because you're married to a hard line philosophy.

    Voting on this poll is easy. The challenge we care about is the quality of the path you forge to get there.

  • Unknown
    Unknown
    edited September 2018

    @Todd

    I would rather see calls to c() remain rather than have them changed to Gdn::config(...).

    Can I ask Why?

    And I feel like you all are discussing bit another topic (I can be very wrong). My post is about recommendations for future for any new code we deliver. But not about create number of epics to pass through current code base and replace calls everywhere.

    I have another questions about how we can organize deprecation process. But this topic is about: "avoid". And I can't understand why "avoid" can't get positive votes. :-)

  • Unknown
    Unknown

    Can I ask Why?

    I explained why in my post. Gdn::config() is a static call to a dependency and that should be done with dependency injection. I would rather use dependency injection there or just leave it as c() if dependency injection isn't going to be used.

    I am most definitely describing new code. The specific sliceParagraph() example came up with new code in a meeting. You said "no global functions" without offering an alternative in the meeting even though we were specifically asking for an alternative. Perhaps there was a misunderstanding there, I don't know.

  • Vanilla Forums
    Vanilla Forums
    edited August 2018

    I believe Kim's question is why c() (an alias for the static call) is preferable to simply making the static call, if we're talking about new code.

    I don't think there's a proposal on the table to start removing c() en masse.

  • Unknown
    Unknown
    edited September 2018

    @Todd

    You said "no global functions" without offering an alternative in the meeting even though we were specifically asking for an alternative

    I don't know what I have to say or to add. Since we already agreed that OOP is our foundation. I did expect that everybody knows the difference between procedural/functional programming vs OOP. There are numerous alternatives to define anything without calling global functions and global context. And for each case developer need to find the best option. I can't see how I can provide any recommendation when we discuss general idea for the project but not particular function.

    I did try to ask you few times: if you want to have deep discussion how to transform particular function: t(), c(), sliceParagraph() - lets open another topic for each and/or every particular function and lets discuss particular function implementation and best way we can transform that.

    but @Linc got me 100% correctly:

    I believe Kim's question is why c() (an alias for the static call) is preferable to simply making the static call, if we're talking about new code.

    (and in addition that is the only thing that c() function does!)

Categories