Skip to content
This repository was archived by the owner on Oct 9, 2018. It is now read-only.

features/modules: Expand modules section #5

Merged
merged 2 commits into from
Jul 8, 2014

Conversation

nathantypanski
Copy link
Contributor

I'd like some feedback on this sketch of modules patterns. I basically covered the simplest and most common design, but I'm sure there's some gaps in the structure here and there's definitely more material available to touch on.

I also think this should mention mod foo { /* ... */ }-style code within a single file. I'm not sure what constitutes a good use case for "inline modules" besides tests, and whether that should be discouraged altogether in favor of "real" modules.

All of these are listed [OPEN], this is really tentative and so on, but any feedback would be super helpful.

@aturon
Copy link
Member

aturon commented Jul 1, 2014

Sorry for the delay in reviewing this, and thanks for another contribution!

I have some overall comments that I'll put here rather than on the diff.

The explanation of mod.rs doesn't seem quite right to me. The mod.rs file is used when you want a module to live in its own subdirectory, which is useful when the module includes several submodules (which can then live in that subdiretory). See http://doc.rust-lang.org/tutorial.html#files-and-modules for more detail on this mechanism.

So using mod.rs is about filesystem organization, not module/API organization.

Also, just to be clear, the imports of a parent module are not visible to its children:

// this is not visible to `inner`
use std::collections::HashMap;

mod inner {
    // this is required
    use std::collections::HashMap;

    pub fn mk_map() -> HashMap<int, int> {
        HashMap::new()
    }
}

While it's true that parent modules often re-export the most important definitions for the top-level (see http://aturon.github.io/style/organization.html for the crate-level version of this), the module hierarchy is also used publicly to organize APIs.

I think a good example to draw from in this section is std::io. It contains several submodules. Some of the submodules, like fs, do not have any children modules and so just live in their own file (fs.rs). Others, like net, have further children, so you have a net subdirectory with a mod.rs defining the net module and several other files for net's children. And since io is itself a module within std, it lives as io/mod.rs and its children modules live under the io directory.

This module structure is part of std::io's interface, which helps keeps it organized. But for convenience, it does re-export several important definitions from its children and defines a bunch of types, traits, and statics directly. (These top-level definitions are then used by its submodules).

So, the top-level points are:

  • Submodules should live in separate files, except for (1) test modules and (2) very short (~ page size) modules.
  • Use foo.rs when foo is a module without any children in separate files.
  • Use foo/mod.rs when foo is a module with children modules in separate files; place foo's children in the foo directory.
  • Use the module hierarchy to organize your API into coherent sections. Define or re-export the most important or common definitions at high levels in the module hierarchy.
  • Consider using internal module hierarchies (including private submodules) to structure your code while hiding implementation details from clients.

Please let me know if the above makes sense. I think that using io as a running example in this section could be really helpful.

@nathantypanski
Copy link
Contributor Author

Thanks for the feedback. I think your points are spot-on and they definitely make sense to me. Some of them seem to be a result of poor wording on my part. For example, when I wrote mod.rs in the first part I really meant "modules" - trying to emphasize that we really shouldn't be putting multiple modules in one file - but it harms comprehension to fudge technical details like that and I'll avoid it.

I agree that io is a good example, and referenced it when working on this. My reason for not using it explicitly was just simplicity - io is big, and I was hoping to condense it down to the useful bits in getting the point across. Probably a more detailed example really is the right thing, since it provides a real working codebase that people can examine to see why things are done this way.

I was pretty busy today (switched apartments!), but I'll put effort into your suggestions tomorrow.

Per Aaron Turon's suggestions, this clarifies much of the wording for
modules.

Additionally, it adds recommendations about:

- Module naming
- Header ordering
- path directives
@nathantypanski
Copy link
Contributor Author

@aturon I've added updates for your recommendations, as well as a couple of other bits that seemed idiomatic - feel free to pick them apart as you please :)

I'll leave the changes as separate commits for now (to preserve discussion history), but I can squash everything together when this is ready if you prefer that.

@aturon
Copy link
Member

aturon commented Jul 8, 2014

Sorry for taking so long to review this -- I've been away on a short vacation. I'm going to merge as-is, and then update with some edits to fix a few minor things.

Thanks again for your ongoing contributions!

aturon added a commit that referenced this pull request Jul 8, 2014
features/modules: Expand modules section
@aturon aturon merged commit d588175 into rust-lang:master Jul 8, 2014
@aturon
Copy link
Member

aturon commented Jul 8, 2014

@nathantypanski FYI, I've pushed some further edits to the section.

@nathantypanski
Copy link
Contributor Author

@aturon Cool, thanks for the update. I think you did a good job with the corrections.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants