0

I am trying to develop a theme engine, which loads themes from a json. I have a Thememanager which is a singleton class and holds a currentTheme variable. I then have a baseViewController which listens to any change in the currentTheme with the help of Boxing technique, and all the viewControllers need to be subclass of base and need to override the observer method to apply their styles. In the box class I have an array of listeners so that multiple view controllers can observer theme change simultaneously, it works well and now my problem is that whenever a view controller gets deallocated, I want to remove that listener also from the box class array of listeners, which I am unable to figure out, because of which listeners are getting piled up.

I tried to write an unbind method in the deint of the viewController and tried to pass the closure like the below but it didnt work

func unbind(listener: Listener?) {

    self.listeners = self.listeners.filter { $0 as AnyObject !== listener as AnyObject }

}

Thememanager

class Thememanager {

    // Hold a list of themes
    var themes = [Theme]()
    // Private Init
    private init() {

        fetchMenuItemsFromJSON()
        // You can provide a default theme here.
        //change(theme: defaultTheme)
    }

    // MARK: Shared Instance

    private static let _shared = Thememanager()

    // MARK: - Accessors
    class func shared() -> Thememanager {
        return _shared
    }

    var currentTheme: Box<Theme?> = Box(nil)

    func change(theme: Theme) {

        currentTheme.value = theme
    }

    private func fetchMenuItemsFromJSON() {

        // TRIAL
        let theme = Theme()
        themes.append(theme)
    }
}

BOX

class Box<T> {

    typealias Listener = (T) -> Void
    var listeners = [Listener?]()

    var value: T {

        didSet {
            for listener in listeners{
                listener?(value)
            }
        }
    }

    init(_ value: T) {
        self.value = value
    }

    func bind(listener: Listener?) {

        self.listeners.append(listener)
        for listener in listeners{
            listener?(value)
        }
    }

    func unbind(listener: Listener?) {

        self.listeners = self.listeners.filter { $0 as AnyObject !== listener as AnyObject }

    }

}

BaseViewController

class BaseViewController: UIViewController {

    private var themeManager = Thememanager.shared()
    typealias Listener = (Theme?) -> Void
    var currentListener: Listener?
    override func viewDidLoad() {
        super.viewDidLoad()
        observeThemeChange()
    }

    override func didReceiveMemoryWarning() {
        super.didReceiveMemoryWarning()
        // Dispose of any resources that can be recreated.
    }

    // Bind the theme variable so that changes are immediately effective
    func observeThemeChange() {
        currentListener = {[weak self] (theme) in
            guard let currtheme = theme else {
                return
            }
            self?.loadWith(theme: currtheme)
        }
        themeManager.currentTheme.bind(listener: currentListener)
    }

    // This method will be implemented by the Child classes
    func loadWith(theme: Theme) {

        self.navigationController?.navigationBar.tintColor = theme.navigationBarTextColor
        self.navigationController?.navigationBar.titleTextAttributes  = [NSAttributedStringKey.foregroundColor : theme.navigationBarTextColor]
        // Need to be implemented by child classes
        print("theme changed")
    }


    deinit {

        themeManager.currentTheme.unbind(listener: currentListener)
    }
}

Theme

struct Theme {

    // Define all the theme properties you want to control.

    var navigationBarBgColor: UIColor = UIColor.darkGray
    var navigationBarTextColor: UIColor = UIColor.black
}
anoop4real
  • 7,598
  • 4
  • 53
  • 56
  • 1
    Why dont you use Notifications to convey the theme change. Add observer for a theme changed notification in your base class and whenever theme changes in the thememanager, just post that notification. – Puneet Sharma Feb 10 '18 at 09:38
  • @PuneetSharma Initial I thought of going with notifications and then found this binding approach and felt better, except this problem. I am using this for viewModel binding also in some other modules rather than using KVO – anoop4real Feb 10 '18 at 09:45
  • The Box object is holding the Viewcontrollers(listeners) strongly in an array. The deinit is never going to get called. It is a case of cyclic dependency. – Puneet Sharma Feb 10 '18 at 09:50
  • @PuneetSharma The box object holds the closures and I have added weak capturing the closure, I am able to see that deinit of viewcontrollers are getting called – anoop4real Feb 10 '18 at 09:54
  • @anoop4real did you find any solution for this? – SleepNot May 04 '20 at 12:22
  • @SleepNot Its been a long time since I worked on this... I will check my archives and see – anoop4real May 04 '20 at 12:42

1 Answers1

0

The issue is with the comparison of closure in unbind method as it snot available for closures and functions(). See this. I guess you could maintain a hashmap where listeners be the value and a unique identifier be the key. It is going to be much faster to unbind it as well.

But, I feel Notifications way is much better as it gives you the same behaviour(Publisher-Subscriber) without you having to manage the listeners.

Puneet Sharma
  • 9,369
  • 1
  • 27
  • 33
  • This will give error "'weak' may only be applied to class and class-bound protocol types" – anoop4real Feb 10 '18 at 10:05
  • If Listener is not an object, how are you comparing it as anyobject in unbind method? – Puneet Sharma Feb 10 '18 at 10:11
  • I guess you typecasted listener to anyobject because compiler must have complained as !== is not available for closures. Thats why I was assuming the listener to be either a pointer object. After looking at the code again, it surely is not a problem of cyclic dependency but the conparison issue of closures in unbind method – Puneet Sharma Feb 10 '18 at 10:13
  • yeah correct, closure comparison issue, I am trying an approach of setting an identifer ....lets see – anoop4real Feb 10 '18 at 10:18
  • @anoop4real: I guess you could maintain a hashmap where listeners be the value and a unique identifier be the key. It is going to be much faster to unbind it as well. But again, I feel this code is error-prone and adds added responsibility of generating unique keys. – Puneet Sharma Feb 10 '18 at 10:24