What are the Slack Archives?

It’s a history of our time together in the Slack Community! There’s a ton of knowledge in here, so feel free to search through the archives for a possible answer to your question.

Because this space is not active, you won’t be able to create a new post or comment here. If you have a question or want to start a discussion about something, head over to our categories and pick one to post in! You can always refer back to a post from Slack Archives if needed; just copy the link to use it as a reference..

Hi , is there any reasons why "UtilTextService" instantiates directly in "PropelSchemaMerg

UKBF1R1S5
UKBF1R1S5 Posts: 34 🧑🏻‍🚀 - Cadet
edited September 2019 in Help

Hi <!channel>,
is there any reasons why "UtilTextService" instantiates directly in "PropelSchemaMerger"? I think a bridge that will be injected over constructor is better solution.

Comments

  • UK5DS29L2
    UK5DS29L2 Posts: 546 🧑🏻‍🚀 - Cadet

    Because it works , I guess

  • UK5DS29L2
    UK5DS29L2 Posts: 546 🧑🏻‍🚀 - Cadet

    IMO - If it works, it's OK. You won't blame people for taking another road to the store, just because you like another road more. Let programmers do it the way it works for them the best. 😉

  • UKBF1R1S5
    UKBF1R1S5 Posts: 34 🧑🏻‍🚀 - Cadet

    I don't want to blame someone else.

  • Unknown
    Unknown

    hi

  • Unknown
    Unknown

    can you point me to some code?

  • Unknown
    Unknown

    and i absolutely disagree with @UK5DS29L2 🙂

  • Unknown
    Unknown

    of course there can always be a shortcut, but it should be a reflected decision. These things can make the life of people further down the road harder than necessary

  • UKN2CGHSB
    UKN2CGHSB Posts: 13 🧑🏻‍🚀 - Cadet
    edited September 2019

    https://github.com/spryker/propel/blob/master/src/Spryker/Zed/Propel/Business/Model/PropelSchemaMerger.php#L249

  • UKN2CGHSB
    UKN2CGHSB Posts: 13 🧑🏻‍🚀 - Cadet 
        private function getElementName(SimpleXMLElement $fromXmlChildElement, $tagName)
    {
        $elementName = (array)$fromXmlChildElement->attributes();
        $elementName = current($elementName);
        if (is_array($elementName) && isset($elementName['name'])) {
            $elementName = $tagName . '|' . $elementName['name'];
        }
        if (empty($elementName) || is_array($elementName)) {
            $utilTextService = new UtilTextService();
            $elementName = 'anonymous_' . $utilTextService->generateRandomString(32);
        }
        return $elementName;
    }
  • UKN2CGHSB
    UKN2CGHSB Posts: 13 🧑🏻‍🚀 - Cadet
    edited September 2019

    change was introduced in https://github.com/spryker/propel/commit/11b7a10f26896b1dd8430c09f9a0842c4387a6ec

    stereomon committed on Dec 5, 2016

  • Andriy Netseplyayev
    Andriy Netseplyayev Domain Lead Solution Architecture Sprykee Posts: 519 🧑🏻‍🚀 - Cadet

    looks like that particular commit was about replacing the library with a service, and not “injecting it properly”. I think question from @UKBF1R1S5 is not to finger-point, but rather to understand something that doesn’t fit to the general picture. And I don’t see a good reason why not injecting it properly..

  • Unknown
    Unknown

    i agree, this was probably a semi-automatic refactoring

  • Unknown
    Unknown

    it is still unclean from Spryker side, should not look like that

  • UKN2CGHSB
    UKN2CGHSB Posts: 13 🧑🏻‍🚀 - Cadet

    sure, the line was touched (for service replacement) ~3½ years ago and nobody was taking any further care to check for new FooService() usages in business models..

    As an answer to original question: "legacy code" could be the only reasonable explanation why proper injection was not used

  • Unknown
    Unknown

    i agree, unfortunately

  • UKBF1R1S5
    UKBF1R1S5 Posts: 34 🧑🏻‍🚀 - Cadet
    edited September 2019

    FYI. I created a pull request by spryker.

Categories