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

Configurable job class #6

Closed
wants to merge 3 commits into from
Closed

Configurable job class #6

wants to merge 3 commits into from

Conversation

KitsuneRal
Copy link
Member

So, here it comes - a class that can facilitate creating new jobs. It's not that you can write SimpleJob(some,construction,configuration) and magically get support of one more action - but I suppose it wouldn't be quite readable anyway. Kindly consider, along with a usual dose of code cleanup (actually, all the interesting stuff is in the last commit).

1. Deduplicate the code creating SyncRoomData structures for different join states.
2. Use anonymous struct instead of QPair<> to improve code readability.
This class encapsulates a simple case when the job result doesn't generate Event* objects. In such cases a SimpleJob descendant won't need to implement parseJson() - instead, it's enough to specify expected result items through SimpleJob::ResultItem<> template. The implementation is backwards compatible - in this commit, PasswordLogin result parsing has been completely rewritten with ResultItem<> without affecting PasswordLogin clients; and RoomMessagesJob partially used ResultItem while still parsing the event list in the class. Support of non-toplevel JSON keys and complex objects (think QList<Event*>) is in the pipeline.
@KitsuneRal
Copy link
Member Author

Looks like I've found a more flexible way using a pImpl idiom. Expect a considerable rewrite of the last commit.

@KitsuneRal
Copy link
Member Author

No more relevant.

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.

1 participant