3 ms·
I think I like the idea, but the structure of the code doesn't look the best. What most sticks out to me is the "Managers" directory. I've seen similar patterns
by LoganDark 11mo ago
I think I like the idea, but the structure of the code doesn't look the best. What most sticks out to me is the "Managers" directory. I've seen similar patterns before, even at my current place of work, but they seem to correlate with less experienced implementations. For instance, I clicked on one of them randomly and already found an issue: https://github.com/nook-browser/Nook/blob/09a4c6957a2e9fd7c5737dc5a6adc01356e2ab9a/Nook/Managers/BoostsManager/BoostsManager.swift#L593-L613 https://github.com/nook-browser/Nook/blob/09a4c6957a2e9fd7c5...
I guess `www.` (and only `www.`) is always special, and the only TLDs with two components are `"co.uk", "co.jp", "com.au", "co.nz", "com.br"`?
I don't know how critical this "Manager" is (what even is a "boost"?), but a web browser should absolutely have a proper list of TLDs!
- normie3000 11mo ago> the only TLDs with two components are `"co.uk", "co.jp", "com.au", "co.nz", "com.br". Is this sarcasm? The public suffix list will give some ideas for omissions: https://publicsuffix.org/list/public_suffix_list.dat https://publicsuffix.org/list/public_suffix_list.dat
- LoganDark 11mo agoThat was me pointing out what was plainly implemented in the code snippet I linked. It is obviously nowhere near the truth.
- monerozcash 11mo ago> and the only TLDs with two components are `"co.uk", "co.jp", "com.au", "co.nz", "com.br"`? There are a plenty of reasons why you might want to treat the most common two part TLDs as special, while not doing that with the less common ones.
- ziml77 11mo agoAnd those reasons would be...?
- valleyer 11mo agoRight; top-level comment is saying that those are all missing from the linked code.
- grodriguez100 11mo agoUh oh. Looks bad.
- SoKamil 11mo ago> What most sticks out to me is the "Managers" directory. I've seen similar patterns before, even at my current place of work, but they seem to correlate with less experienced implementations What is wrong with such structure? How would you structure this code? Genuinely asking
- LoganDark 11mo agoThere are no peer-reviewed studies yet for me to corroborate this with, but I've seen this pattern primarily from a specific type of autistic, and it's similar to an actor pattern: a Manager is expected to entirely "manage" whatever feature it's concerned with. This is usually different from a simple module by not collecting related functionality regarding the feature, but rather trying to contain the entire feature itself. This typically creates artifacts like each "Manager" owning too much of its implementation (not benefiting from or contributing to shared structures, such as a proper domain suffix list), inconsistency between different parts of the app (since different "Managers" don't necessarily share common patterns between them), and tons of hooks into random "Managers" all over the code. To me, it feels a bit like an "emotionally driven" architecture, where the organization of the code is based on the list of features of the app, and not based on the implementation of those features. So rather than having, for example, a drag and drop component for the tabs to use, you would have, for example, a ReorderingTabsManager, and the implementation may behave differently than drag and drop in other places. So rather than factoring out code into modules for deduplication, you're making modules ("Managers") based on where they are in the product, and duplicating functionality across each module, to varying standards of completeness and/or quality. Now I don't know if this project is quite that egregious, but it hopefully illustrates why I raise an eyebrow when I see a project architected this way.
- BigBoiKahuna 11mo agoWhat the hell are you talking about?
- LoganDark 11mo agoI don't mind explaining, but can you elaborate on what part of my comment is confusing to you?