FOSMessage icon indicating copy to clipboard operation
FOSMessage copied to clipboard

Can't extend sender object

Open iBasit opened this issue 9 years ago • 9 comments

I wanted to extend the sender object to overwrite existing methods to match with our business specs.But I can't do, because lots of methods and properties are private. can you please change it to protected so it is easier to overwrite.

I can re-write that to have same effect, but it was best that if it can be avoided.

iBasit avatar Jan 28 '16 13:01 iBasit

That's a good idea. I don't have time right now, I'll do it this night. If you need it now, please feel free ot open a PR :) !

tgalopin avatar Jan 28 '16 13:01 tgalopin

I would also do it for the Repository and the Tagger btw.

tgalopin avatar Jan 28 '16 13:01 tgalopin

I thought a bit about this and I'm not sure it's a good idea after all. I think a complete event system should be enough to do anything want. I'll improve the current event system. Tell me if you don't have anything you need.

tgalopin avatar Feb 01 '16 16:02 tgalopin

I changed a bit the events, for more precision in targeting a given event. Instead of general PRE_PERSIST and POST_PERSIST, you have now 4 events: https://github.com/FriendsOfSymfony/FOSMessage/blob/master/src/EventDispatcher/EventDispatcherInterface.php

I know this will probably break your code but it's quite easy to fix: just use these constants instead of the old ones. You can check the Sender if you want to know when these events are dispatched: https://github.com/FriendsOfSymfony/FOSMessage/blob/master/src/Sender.php#L54.

tgalopin avatar Feb 01 '16 16:02 tgalopin

Well honestly, I think adding extra event to make precision targeting is much better solution and improvement.

But it wont hurt to have private methods or properties changed to protected. It gives full flexibility, just for sake of it.

When I overwritten sender class, it gave me couple of issues. So I had to copy all of it, which I didn't wanted to do. but at the moment messaging system is done for us. Using our sender class for now (recommend others to use events).

  • attached messaging system with firebase to make it live chatting.
  • will add email message on top. (pending)

iBasit avatar Feb 01 '16 18:02 iBasit

But it wont hurt to have private methods or properties changed to protected. It gives full flexibility, just for sake of it.

Actually, the problem is that once I declare a property or a method as protected, I need to keep it until the next major version as I'm not allowed to BC break in minor ones. That's why I'm not so much a fan of setting properties as protected.

Is there something you currently can't do with the event system? It's a much better solution for extendability.

tgalopin avatar Feb 01 '16 18:02 tgalopin

I can't really think of anything that event can't do.

iBasit avatar Feb 01 '16 18:02 iBasit

I think I'll create protected getters in the sender. I'll think about it to do something useful but still maintainable.

Anyway, thanks very much for your feedbacks! If you are interested, I made a demo application using Symfony 2.8 and FOSMessage: https://github.com/tgalopin/FOSMessage-demo. It let me see how we could create a useful bundle in the future.

tgalopin avatar Feb 01 '16 18:02 tgalopin

If you do create that, which will also be a plus point.

But you will also have to make other methods protected as they are private, which caused our code issues, so I had to also copy private methods.

iBasit avatar Feb 02 '16 15:02 iBasit