Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve mapRelatives in BaseObject #49

Merged
merged 1 commit into from
Nov 25, 2015
Merged

Improve mapRelatives in BaseObject #49

merged 1 commit into from
Nov 25, 2015

Conversation

alexsoft
Copy link
Contributor

@irazasyed mapRelatives doesn't really supports initializing inner properties
So for example we get webhook update. Message is transformed to object of Message class, but 'from' and 'chat' fields are not.
This code fixes it. I am not sure if you need that continue, but second if does all the job.

mapRelatives doesn't really supports initializing inner properties
So for example we get webhook update. Message is transformed to object of Message class, but 'from' and 'chat' fields are not.
This code fixes it. I am not sure if you need that continue, but second if does all the job.
@alexsoft alexsoft mentioned this pull request Oct 24, 2015
@irazasyed
Copy link
Owner

Hi @alexsoft,

Thanks for opening the PR.

The way mapRelatives works is basically, the inner properties are initialized on-demand. I don't see any point in initializing all the inner properties and mapping them when you don't even need it.

Here's an example of how it would work with a webhook setup:

$update = Telegram::getWebhookUpdates();
$message = $update->getMessage(); // Maps to Message Object
$from = $message->getFrom(); // Maps to User Object
$chat = $message->getChat(); // Maps to Chat Object

Edit: For webhook this is definitely a good improvement and can be considered applying, But for manual fetching with maybe a good chunk of backlog messages, I'm concerned about its performance. Let me test this and I'll merge soon. Thanks!

irazasyed added a commit that referenced this pull request Nov 25, 2015
Improve mapRelatives in BaseObject
@irazasyed irazasyed merged commit 57f6ced into irazasyed:master Nov 25, 2015
@irazasyed
Copy link
Owner

I tested and it's not much of a difference but the improvement is still good. I noticed we could also use get() method from the collection API and still have the object properly mapped. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants