andreanmasiro / dogs-challenge Goto Github PK

I think this property don't need to be open, I did a quick search in the app and the only class access it is the AppDelegate and I could see we do not need to open "for now".

So I think it should be:

private var appCoordinator: AppCoordinator!

Maybe In the feature other modules of app will want access it and the access should be changed to:

private(set) var appCoordinator: AppCoordinator!

But for now we should not presume the feature and make it simple, and focus the main issue, to not bring complexity to code.

This is not only for this part below but is too a topic about classes declarations in general:

The final statement at the begin of the class declaration. It's important for us because indicates:

• The Class is not open for modification 😝.

• The developer not planned this to work with inheritance and to open it maybe we should understand what's going on and make some modifications if needed.

• In the other hand (in the rainbow world), if the class not use this statement (final) I could trust on the Dev and know when this class was created he thought in the inheritance of it.

We can discuss this per skype or message, because it is a very long topic.

Not most important, but has your shine!

It helps the compiler make optimization to your class. Run time says: Thanks! hahahahah.

To see this information read:

http://blog.human-friendly.com/the-importance-of-being-final

I think Swift language should adopt the final statement as default like Kotlin:

By default, all classes in Kotlin are final, which corresponds to http://www.oracle.com/technetwork/java/effectivejava-136174.html">Effective Java, Item 17: Design and document for inheritance or else prohibit it."

There are a lot of advantages =)

Hey,

I don't know witch version of cocoa pods you are using, this can cause problem of compatibility to the project Podfile when I execute the pod install.

I see the App initialization, you guarantee that the window will not be empty, so I believe you do not need to give the nullable taste to this variable. hahaha

If your concern is with the app crash I believe you can guarantee in tests.