By doing this, we can lazy-attach the backend object. This can help prevent circular references, but also ensure better internal behavior of objects that may not have been built correctly. Doing this requires adding a prop that with the internal backend type string to all VersioncontrolEntity classes, though, and requiring all backend classes to specify it in their class defs (currently only VersioncontrolRepository has to, as its a db field).
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | replace_all_literal_entity_backend_instances_with_entity_getBackend-1045464-3.patch | 5.77 KB | cvangysel |
Comments
Comment #1
sdboyer commentedPlus, this'll make it NOT a huge pain in the ass to
print_r()on a VersioncontrolEntity object.Comment #2
marvil07 commentedcleaning api is a good thing ;-)
Comment #3
cvangysel commentedProposed patch in attachments.
Comment #4
marvil07 commentedGreat!
Taking a look at the patch :-)
Comment #5
marvil07 commentedAfter taking a look, I think the problem is the title of the issue.
This patch does the right work on replace, but the target of the issue is not well reflected on the title.
AFAIK the missing part here is about re-factor VersioncontrolEntity to remove
$backendprotected data member and use agetBackend()method to actually use the backend. This implies storing the backend string name on theVersioncontrolEntityclass.In the other side, now that I think a little more about it, I do not see the use of that re-factor, since I can not recall a place where we do not have the backend loaded(using lazy loading is about not having them loaded all the time), since they stay as static variable on
versioncontrol_get_backends()function, and we always use that function somewhere in the flow.I see the
print_r()reason logic, but if you use devel'sdpm()it prints only public data members, so not a problem really.@sdboyer: So, do you still think this is a good idea? IMHO this is not really necessary.
Comment #6
sdboyer commentedYes, I still think it's a good idea. It's been a while since I originally thought through this, I but I believe it had to do with making it object instanciation cheaper at some crucial point.
Regardless, having a getter for something like this is almost never a *bad* idea. It decouples code.
Comment #7
sdboyer commentedCommushed. thanks!