@nikosdion
On 24 Jan 2015 12:11, "Quibizo" notifications@github.com wrote:

Steps to reproduce the issue

Hi,

totp.php uses deprecated JFactory::getURI() function at line 128. I think
that it should be changed to JUri::getInstance() as soon as possible to
avoid future conflicts.

Thanks a lot
Expected result Actual result System information (as much as possible)

Joomla 3.3.6
Joomla Platform 13.1.0
Additional comments

I think that line 128 at totp.php should be changed from:

$hostname = JFactory::getURI()->getHost();

to:

$hostname = JUri::getInstance()->getHost();

—
Reply to this email directly or view it on GitHub
#5883.

That is removing a dependency with another one, doesn't makes so much sense. We need to use proper dependency management. With JFactory the spots are easier to find.

@rdeutz

That is removing a dependency with another one, doesn't makes so much sense.

The suggestion is what we write to the log? Can you explain what is the issue with the new code?

https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/factory.php#L480

There is no point changing this now, anywhere in Joomla!'s code. JUri will also be replaced by a namespaced class which will be extending the Framework's Uri class (because the framework can't handle $live_site). Until that happens JFactory::getUri won't be obsolete. Moreover, the deprecated JFactory methods cannot be removed until Joomla!'s application use a DI container which further means that any change today is completely futile.

So, -1 from me.

Thank you for your comments. Based on them I am closing this issue.

@zero-24 it doesn't makes it better. JFactory::getURI() is the same as JUri::getInstance() both calls are bad for testing because the call uses a class name and you don’t have a change to mock that for testing.

I don't think it is a good suggestion, it doesn't fix the real problem.

@nikosdion Regarding this:

JUri will also be replaced by a namespaced class which will be extending the Framework's Uri class (because the framework can't handle $live_site).

The Framework's URI package is already integrated and JUri is extending it for 3.4. The JUri class itself isn't going anywhere anytime soon, at least until a plan is made regarding whether CMS classes should be namespaced or not and how that transition will be managed.

As for the JFactory::getURI method being deprecated, IMO it's one of those methods in the factory that you could do away with and not hurt the system. Like @rdeutz mentions it isn't any better to use the method it calls because of the lack of testability, but removing JFactory::getURI is nowhere near as catastrophic as suggesting to remove JFactory::getApplication

I agree. I also venture that it doesn't make any sense to touch any JFactory method calls until Joomla! uses a DI container. In this case we'll only need a way to get a reference to the DI and/or the application.

In fact, since @rdeutz talked about testing, it was much easier to test code when we were all using JFactory::getURI. One (really bad, global, full of evil static calls) class to mock. I think at some point sooner rather than later we need to have the Joomla! 4 talk. We can't go on deprecating stuff without having a solid plan for moving forward. Deprecating a bad practice and replacing it with another bad practice is futile having the net effect of making developers' lives miserable and increasing the cost of code maintenance without any benefit whatsoever. This amount of developer time would be better spent coding a better architecture with tangible benefits in unit testing, maintainability and so forth.

JFactory::getURI() has been deprecated. I don't think that using a deprecated function is a good practice (unless you want to keep your code unmaintained).

I really don't understand the reasons for keeping an outdated function. It has been replaced in core scripts (I can find any use of JFactory::getURI there). So I think that it should be replaced in core plugins too.

Talking about testability or future developments is out of scope. The current state of the project states that JFactory::getURI is deprecated, for better or worse. So I think that we must code according the current rules to keep the coherence of the project. If tests must be changed because of that, they have to be changed. It cannot be avoided. Well... since the function is deprecated, it can be avoided for a while.

Of course, this is a futile issue. The code is still working and will probably work for a long time.

Thanks for your help.

Currently the two factor authentication also depends on FOF which will be removed from Joomla! in version 4. This means that this feature will be rewritten. There's no point changing something today when the code has to be rewritten tomorrow, more so when there's no benefit whatsoever. We'd have to rewrite the code in the context of Joomla! 4 to make any sense. But since there's no roadmap for Joomla! 4 this is also a moot point.

The code is NOT unmaintained! It is well maintained and working fine. It will HAVE to be rewritten anyway. And for crying out loud, we've spent 2 hours discussing a non-issue when there are other, real issues to work on. Can we please stop discussing a non issue? It's a waste of everyone's time :(

I know it is hard to tell when you get emails from the tracker but this issue has been closed