Image scraper using Dependecy Injection in Rust with generics

Question

I'm writing a scraper that downloads images from a given subreddit. I'm new to Rust, and in the majority of my previous projects I was using C++ or Java, where it's easy to do Dependency Injection.

I wanted to use DI in this project, so I could inject a mock service for tests, and because I think that's how it's supposed to be done - but maybe that's not the case, and one should use a totally different design pattern in Rust?

My code (I'm on an early stage of development):

reddit_service.rs:

pub trait RedditService {
    fn get_next_urls(self) -> Vec<String>;

    fn from_new() -> Self;

    fn get_one_url(self) -> String;
}

pub struct MockRedditService {}

impl RedditService for MockRedditService {
    fn get_next_urls(self) -> Vec<String> {
        Vec::new()
    }

    fn from_new() -> MockRedditService {
        MockRedditService {}
    }

    fn get_one_url(self) -> String {
        String::from("example url")
    }
}

manager.rs:

use super::reddit_service::RedditService;

pub struct Manager<RS> {
    reddit_service: RS,
}

impl<RS: RedditService> Manager<RS> {
    pub fn new(reddit_service: RS) -> Manager<RS> {
        Manager { reddit_service }
    }

    pub fn scrap(self, subreddit: &str, limit: u32) -> () {
        println!("Scrapping {} images from {}", limit, subreddit);
        println!("Scrapped: {}", self.reddit_service.get_one_url())
    }
}

and in main.rs:

let app_manager = Manager::<MockRedditService>::new(MockRedditService::from_new());
app_manager.scrap(subreddit, limit);

My two questions are:

1. is that how DI is supposed to be done in Rust? (I've also read about using Box<dyn Trait>, but I didn't like it)
2. is the DI even useful here? Maybe there's a better way of achieving clean code, which is easy to test, without using it?

Answer 1

is that how DI is supposed to be done in Rust?

The core concept of dependency injection is: don't make your data/service dependencies hard-coded, but provide some way for the caller/configuration to provide an alternative. You've identified two ways to do it — trait objects and generics, both of which start with a trait, i.e. polymorphism — and those are both fine but you don't necessarily need to use polymorphism to do dependency injection.

In particular, since your application is a kind of network client, one straightforward approach to testing is to direct your client to connect to a different server (in particular, one that you started up locally, that pretends to be Reddit). Thus, instead of providing a different type of implementation of RedditService, you provide a different server address.

Advantages of using a local server for testing:

• The client code is not constrained by needing to pass through the specific RedditService interface you designed — it can be structured however is convenient for its actual job, and refactoring that structure does not require modifying the tests.
• You are exercising more of the stack — not just your scraping logic, but also the HTTP client you use. When writing mocks of the interfaces between individual “objects”, it can be very easy to write a test that technically has 100% code coverage, but is not actually useful because the behavior of the mock does not accurately represent the behavior of the real subsystem it's meant to imitate.

Disadvantages:

• Writing a test-double server probably requires providing more fake data and other API details than implementing RedditService.
• You're testing a larger unit of code, so it's harder to nail down exactly where a failure is.

---

Now, commentary on your actual code:

pub trait RedditService {
    ...
    fn from_new() -> Self;

• Such a constructor is usually called new(), not from_new().

• It is often not a great idea to include zero-argument constructors like this in traits. By including it, you're requiring that every implementation can be constructed from zero information, which is a bad idea especially for a network client:

  • The real implementation should probably be constructed with some configuration (server address, throttling policy, User-Agent string, etc.)
  • The mock implementation should probably be constructed with some test data. So, remove this method entirely, and just let each individual implementation have an inherent new() that takes whatever parameters are useful.

  (In cases where a zero-argument constructor makes sense, consider using the standard library's Default trait for that purpose.)

    fn get_next_urls(self) -> Vec<String>;
    fn get_one_url(self) -> String;

These two methods take self by value, meaning that once you call them you can't use the value you called them on any more. They should probably take a reference instead:

    fn get_next_urls(&mut self) -> Vec<String>;
    fn get_one_url(&mut self) -> String;

This isn't a matter of style, though: it's something that you will find you need to change one way or another to make your code actually work.

    pub fn scrap(self, subreddit: &str, limit: u32) -> () {

-> () is the default return type; you can leave those symbols out entirely. The Rust toolchain includes a linter that will tell you things like this: run cargo clippy and see what it says. Clippy lints (as opposed to compiler built-in lints) are not always good ideas; they're deliberately more ambitious and more likely wrong than compiler lints. However, they are designed to tell you about a lot of possible issues that you can run into a beginner, so I recommend reading what they have to say.

---

Finally, a point of English rather than Rust: The tool you're writing is a scraper to scrape Reddit, not a scrapper to scrap Reddit. They are both standard English words but they mean different things.