Consider making the configuration, initialPopupData and reEnablePopupData public.

[VincentSit]

Hi, thanks for sharing this library.

I have a question. I just want to change the frequency property of the configuration, but write a lot of unnecessary duplicate code. It doesn't seem elegant enough.

The ArekLocalizationManager is also not public, so it is not possible to construct data using it.

I wonder if we can make them public so that we can easily change the values.

func askForPermission() {
    let tmp = ArekNotifications()
    let configuration = ArekConfiguration(frequency: .OnceAWeek, presentInitialPopup: true, presentReEnablePopup: true)
    let initialPopupData = ArekPopupData(title: R.string.localizable.arekNotifications_initial_title(),
                                         message: R.string.localizable.arekNotifications_initial_message(),
                                         image: "\(tmp.identifier)_image",
                                         allowButtonTitle: R.string.localizable.arekNotifications_allow_button_title(),
                                         denyButtonTitle: R.string.localizable.arekNotifications_deny_button_title(),
                                         styling: nil)
    let reEnablePopupData = ArekPopupData(title: R.string.localizable.arekNotifications_reenable_title(),
                                          message: R.string.localizable.arekPhoto_reenable_message(),
                                          image: "\(tmp.identifier)_image",
                                          allowButtonTitle: R.string.localizable.arekNotifications_allow_button_title(),
                                          denyButtonTitle: R.string.localizable.arekNotifications_deny_button_title(),
                                          styling: nil)
    let arek = ArekNotifications(configuration: configuration, initialPopupData: initialPopupData, reEnablePopupData: reEnablePopupData)
    arek.manage { Log.info("APNs permission is \($0)") }
  }

[ennioma]

Hi @VincentSit,
about the configuration, I agree that I could change the init to this:

public init(frequency: ArekPermissionFrequency = .OnceADay, presentInitialPopup: Bool = true, presentReEnablePopup: Bool = true)

so you can instantiate it only passing the ArekPermissionFrequency.

About the ArekLocalizationManager, I preferred to push for a convention described here. How could it help you if we make the ArekLocalizationManager public?

Thanks for using the library!

[VincentSit]

Hi @ennioma , thanks for response.

I just noticed that three parameters in init are optional, I can totally omit them. What a stupid mistake. so now I don't have any problems.

BTW, I think you add default value to those parameters is good.

[VincentSit]

Hi @ennioma ,

If I ignore the remaining two parameters, they will not be created automatically, you can see the call stack.

I think this call stack may not meet the expectations of the code design, init(identifier:) is not called, initialPopupData and reEnablePopupData are not created. The coalescing operator doesn't make sense here.

public init(configuration: ArekConfiguration? = nil,
                initialPopupData: ArekPopupData? = nil,
                reEnablePopupData: ArekPopupData? = nil) {
        
        self.configuration = configuration ?? self.configuration
        self.initialPopupData = initialPopupData ?? self.initialPopupData
        self.reEnablePopupData = reEnablePopupData ?? self.reEnablePopupData
    }

[ennioma]

I think the stack is correct because the function signature is init(configuration:initialPopupData:reEnablePopupData). Could you please show me how do you call that?

[VincentSit]

Hi @ennioma ,

let configuration = ArekConfiguration(frequency: .OnceAWeek, presentInitialPopup: true, presentReEnablePopup: true)
let arek = ArekNotifications(configuration: configuration)
arek.manage { Log.info("APNs permission is \($0)") }