Timeline for Unit testing private methods
Current License: CC BY-SA 4.0
8 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Aug 18, 2020 at 2:13 | comment | added | Alexander |
"In fact the newest one on the team should have final say on how understandable a name is." I agree, I go by that matra for the most part, but there's gotta be some minimal bar of understanding. I would oppose fileSuffix(forDomain: domain, ext:"plist"). What does it mean to do fileSuffix(forDomain: domain, ext: "png")? There is no file suffix for other exts, that fact that your looking for a preference list in particular is (and not cat picture) absolutely fundamental to the lookup, it's literally the entire purpose of the class.
|
|
| Aug 18, 2020 at 1:54 | comment | added | candied_orange |
@Alexander-ReinstateMonica fileSuffix(forDomain: domain, ext:"plist") would be less hardcoded and more descriptive. You can make the ext an overridable default if plist really is that typical. Please don't use "It's very common in that domain" as an argument against a plea for a better name. It's easy to end up hazing the newbies. In fact the newest one on the team should have final say on how understandable a name is.
|
|
| Aug 17, 2020 at 15:29 | comment | added | Alexander |
the p isn't a fixed prefix, it's part of the established domain term "plist". I understand that bad names hurt, you're preaching to the choir. I'll reiterate: Is that good enough justification to keep it? I don't really know what a better alternative would be, preferenceListName(forDomain:)? Do you have any suggestions?
|
|
| Aug 17, 2020 at 15:26 | comment | added | candied_orange | @Alexander-ReinstateMonica txt is not just a niche file extension but a method that made a filename end in timestamp.txt should not be called pTxtName. Seriously, bad names hurt. | |
| Aug 17, 2020 at 13:06 | comment | added | Alexander |
"Plist" isn't just a niche file extension like say say .stl (which non-obviously stands for "stereolithography", which is a 3d modelling format used for CAD), it's a very common term in macOS/iOS development that would familiar to anyone in that domain. Is that good enough justification to keep it? I don't really know what a better alternative would be, preferenceListName(forDomain:)? Do you have any suggestions?
|
|
| Aug 17, 2020 at 12:55 | comment | added | candied_orange | @Alexander-ReinstateMonica “.plist” is a hard coded constant. The logic here doesn’t even care about it. It cares about replacing the domain value. Yet you elevate the value of the hard coded value to a name. Seriously, pick a name that makes this meaningful to others. Don’t make me look inside to understand what you’re trying to do. | |
| Aug 17, 2020 at 12:45 | comment | added | Alexander |
I mean, plistName? Come on. Keyword args are part of the method name in Swift, so the actual method name is plistName(forDomain:), which is a very typical/conventional naming style in Swift. See for example FileManager.attributesOfItem(atPath:). I think you and the other answer conflict (they make a lot of points, but their overall conclusion is that I should extract this, and I'm not sure how to square the two)
|
|
| Aug 17, 2020 at 6:42 | history | answered | candied_orange | CC BY-SA 4.0 |