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

Added serialization / deserialization ability for AppScreen #144

Closed
wants to merge 1 commit into from

Conversation

Starksoft
Copy link

@Starksoft Starksoft commented Feb 17, 2021

Hi!

I'm using Cicerone 5.0.0 and in this version serialization worked fine. After update to 6.6.0 I found, that serialization doesn't work. Serialization saw broken by AppScreen and Creator<A, R>.

Also naming of functions in Screens object starts with upper-case letter and lint warns us about it.
Снимок экрана 2021-02-17 в 11 02 18
I suggest to change functions to val and user UPPER-case naming

@Starksoft Starksoft reopened this Feb 17, 2021
@terrakok
Copy link
Owner

terrakok commented Mar 3, 2021

First of all, In case when you have to pass some arguments to new screen you should use function with arguments. So I like consistency and I use functions for all my screens.

About 'serializable'. Why do you need it? Basically using serializable interface is bad idea. Nonetheless, if you want it you have to serialize data but not screen instance. Because Screen is factory for fragment (or activity).

@Starksoft
Copy link
Author

First of all, In case when you have to pass some arguments to new screen you should use function with arguments. So I like consistency and I use functions for all my screens.

It's, Ok, but in functions first letter must be lower-case.

About 'serializable'. Why do you need it? Basically using serializable interface is bad idea. Nonetheless, if you want it you have to serialize data but not screen instance. Because Screen is factory for fragment (or activity).

In my app I have two Activities: MainActivity and CommonActivity.
When I need to open Fragment in other Activity, I need to pass Screen instance (with argument or not) through Intent, for that case I need to Serialize Screen. When Cicerone was written in Java, I can create my own child-class and implement Serializable interface. But when You migrate to Kotlin You add a Creator interface and Serialization now is broken.
If You don't want to add Serialization to base classes, we can create a SerializableAppScreen.

@terrakok
Copy link
Owner

terrakok commented Mar 3, 2021

in functions first letter must be lower-case

Do you know about Compose? 😄
изображение

For your case i recommend you pass not serialized screen but serialized parameters for screen. And on other activity side select screen by received parameters

@Starksoft
Copy link
Author

Cicerone is not the Compose. And it doesn't use DSL.

No, we need to pass serialized Screen and we don't want to implement odd logic with selection of screen by screen args.
We'll continue to use forked Cicerone.

@terrakok
Copy link
Owner

terrakok commented Mar 3, 2021

In my opinion, description of screens is simple DSL. Cicerone doesn't force to use upper case for first letter. You can do it as you want 😉

You can use own fork for your cases with serialization of screens.
I hope I showed that Cicerone doesn't stop you for using multi activity approach but starting new app development I would recommend you Single activity approach

@Starksoft
Copy link
Author

Our project is a bit larger, than example 🙂

In our project we can't use single activity in case of different themes and other platform staff, like other processes

@terrakok
Copy link
Owner

terrakok commented Mar 3, 2021

I think You are wrong if you think that Single activity approach is only for small applications.
e.g. Uber, Telegram and lots of other big and popular applications are written in this approach.

But it is offtop for this topic :)
If you interested I leave here this link: https://www.youtube.com/watch?v=wcdqoTubPrU

terrakok added a commit that referenced this pull request Apr 17, 2021
@terrakok
Copy link
Owner

82256a9

Now, FragmentScreen and ActivityScreen are interfaces so you can implement them with Serializable

@terrakok terrakok closed this Apr 17, 2021
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.

None yet

2 participants