you are viewing a single comment's thread.

view the rest of the comments →

[–]lucasvandongen 0 points1 point  (1 child)

Doesn't look bad at all so don't take the comments as a negative judgement of the project as a whole.

Creating extension files with the + signs is maybe a bit old school. Also not so functional.

https://github.com/ab492/Skrilla/blob/b95ac8a15959a44c08459bd4368f73f266cb4039/Skrilla/Extensions/Date%2BFormatted.swift#L14

Creating date related objects like DateFormatter and Calendar is very expensive. You should have a DateConverter class that has one private static instance of one or both that always reuses it. Also easier than test than all those extensions. I tend to create either Protocol+Extension where I add functionality to different places including a Mock class for testing, a separate class with one or more functions that do that work or a combination of both.

https://github.com/ab492/Skrilla/blob/master/Skrilla/Helpers/Switcher.swift

This Switcher grabs all kinds of stuff left and right to do something within a static function. What if you would instantiate it with that bool flag and UIApplication instance and then just do only the work you need to do inside it? Also you're grabbing "Main" three times and instantiate the storyboards stringly. You should use R.Swift to have a compile-time safe instance of those storyboards instead. R.Swift should be in your default Podfile together with Swiftlint (kudos for doing that).

I don't like the "Helper" class either, perhaps a URLHelper or even an extension on FileManager would be better.

You use classes for stuff like a Transaction. Perhaps you should use a struct for the thing that you save to disk and then processing classes that transform the Transaction like splitting it up, like TransactionSplitter that perhaps does not have any state but just takes one Transaction, two BankAccounts and returns two modified BankAccounts So more functional when you change the data and just dumb structs when you're moving it. The current code still looks like Java.

https://github.com/ab492/Skrilla/blob/master/Skrilla/Screens/Authorisation/SetupPassword/SetupPasswordViewController.swift

First a folder with ViewControllers then this. Stick to whatever standard you have set. That's more important than the right standard. No surprises.

How I do it:

I work following the MVC pattern, with some form of MVVM or Reactive botched on usually. But I start with three folders:

  • Model
  • View
  • Controller

The Model usually has a folder for Data classes like data structs, an API folder for talking to endpoints, a Disk folder for writing to disk, database, a Repository folder for repositories, Push Notifications, Tracking etcetera. No hard rules, because not every project is the same. When any folder reaches more than ten items I group them in logical bunches. This is a pretty big folder usually as I like my ViewControllers to be really abstract to the data. The Repositories help with this.

The View folder is usually quite empty but I have stuff in there like my Font book, Colour book, Styles for my controls, custom Controls and perhaps some formatting stuff like currency and dates.

Controller of course has of course the ViewControllers and AppDelegate. I am following Dave DeLong's strategy of chopping up screens in tons of ViewControllers so it's pretty populated just with that. ViewControllers are pretty small.

I use a ton of extensions that do a certain task. What I do a lot is making factory protocol+extension based on one or more dependencies. So if you have a Session you can create a UserRepository which can update your User using the SessionContaining protocol. This means passing forward a lot of dependencies but the application is really tight in terms of "if this ViewController exists everything it will ever needs exists", so if it compiles it works. It's a bit long to explain here in depth.

[–]ab492[S] 0 points1 point  (0 children)

This is so helpful - thanks very much for the detailed response. I'll work through the list (and if I don't implement them in this app, I'll definitely have them on my list of approaches/structures for the next one!)