0

Method code like so:

- (void)downloadSomething:(NSString *)url {
    Downloader *downloader = [[Downloader alloc] initWithUrl:url];
    NSData *data = [downloader download];
    FileCache *cache = [[FileCache alloc] initWithFile:@"download.cache"];
    cache.data = data;
    [cache save];
}

I think I should mock Downloader and FileCache to verify if it works well. I have thought about change signature like that: downloadSomething:(NSString *)url downloader:(Downloader *)downloader cache:(FileCache *)cache, but it seem to have to do much work before call this methods, that's not what I want.

I am using ocmockito.

Besides, is there a guide to make writing code more testable ?


edit: 2017-01-16 14:54:23

Is this a good idea to write two method like:

- (void)updateCacheWithUrl:(NSString *)url 
                downloader:(Downloader *)downloader 
                 fileCache:(FileCache *)fileCache; // for testing
- (void)updateCacheWithUrl:(NSString *)url; // call above method with (url, nil, nil);
ccnyou
  • 337
  • 2
  • 11

1 Answers1

1

When a collaborator is instantiated inside a method, this creates tight coupling. The difficulty of testing this leads us to explore other designs.

One way is to pass them in. This is what I'd normally do. Then I would make a simpler version that provides default objects for production code.

But in your example, the url is passed to the Downloader, which makes this harder. This suggests that the current design of downloadSomething: violates the Single Responsibility Principle. It's doing two things: downloading, and caching.

So splitting up these responsibilities would probably make things easier to test.

Jon Reid
  • 20,545
  • 2
  • 64
  • 95
  • Maybe I should rename method to 'updateCacheWithUrl:' – ccnyou Jan 16 '17 at 05:35
  • I have thought a lot. Wherever I wrote `Downloader *downloader = [[Downloader alloc] initWithUrl:url];`, it will make that method hard to test. Should I wrote it in private method and do not write test for it ? – ccnyou Jan 17 '17 at 06:59
  • @ccnyou If you make Downloader in one method, I'd pass in a block parameter that acts as a factory. If it's across multiple methods, change it from a parameter to a property. – Jon Reid Jan 18 '17 at 00:57
  • I wonder what's Single Responsibility Principle boundaries ? Is download method that working for new downloading task and continue exists task(same url) violates the Single Responsibility Principle. If it it, should I split up to two public methods or (one public + tow private) methods ? I don't want to check if task exists before call this download mehtod. – ccnyou Jan 19 '17 at 02:55
  • @ccnyou Don't try to overthink it. The important thing is to make progress, even if the path is flawed. In the process, you'll learn more. – Jon Reid Jan 19 '17 at 05:26
  • So I am trying to learn more by asking some one like you who is experienced in it. /smile – ccnyou Jan 19 '17 at 06:43
  • I'd add a block property: `@property (nonatomic, readonly) Downloader *(^makeDownloader)(NSURL *);` set by the initializer. You can then have a simpler initializer that provides a default. For an example, see http://qualitycoding.org/tdd-deferred-design/ where I do MD5 calculation. First I start with by isolating it in a method that a test can override. Then I refactor away from subclass-and-override by injecting a block instead. – Jon Reid Jan 19 '17 at 17:22