Prefer App::default() over App::new() in doc, examples, test

[martinfrances107]

Objective

We are currently following a convention that I see repeatedly across the crates.io ecosystem.

We provide a wrapper App::new() as a proxy for App::default()

This comes from a RUST convention that default is the preferred constructor (when no additional parameters are required).

As a matter of library design we provide App::new only for the flexibility of permitting the game engine developer's whim.

This PR takes the line that when documenting the codebase, when showcasing examples or when testing the codebase we should used the preferred constructor as a way of nudging the reader down that path.

Solution

• Global Replace of App::new() for App::default()
• Visual inspection of PR to ensure only pertinent changes are made.

Testing

• Lots of tests have been updated, no change in functionality

[martinfrances107]

This may be an unrelated issue

as locally - this passes, and fails on the github

cargo run -p ci -- lints

Hmm I will ask on discord.

[martinfrances107]

here is my observation

cargo test --workspace --doc --

running this locally on this PR was passing

I noticed the CI tool using pull in the latest rustc ( dated today 7Aug )

so I ran

rustup update

and now all the test a failing locally, with the same problems as identified by CI

To confirm this I checkout out main .. ran the docs and saw the new error without this patch applied.

So the CI testing pipeline is broken at the moment.

[james7132]

This comes from a RUST convention that default is the preferred constructor (when no additional parameters are required).

I'm going to ask for a citation on this. Where are these convention/guidelines coming from? Though it seldom matters in this particular case, Default::default() cannot be const (yet). Changing this would also likely be advised for all ecosystem crates' documentation too.

[SludgePhD]

This isn't really a widely agreed-upon convention to my knowledge.

Every single data structure in the standard library provides a new() constructor in addition to the Default impl, and all the associated examples use X::new() instead of X::default(). Same goes for most of the popular data structure crates on crates.io.

[Freyja-moth]

Clippy will warn you if you create a constructor with no parameters without a default implementation, which would indicate that having both new and default is the standard.

clippy::new_without_default for those interested.

[martinfrances107]

The best reference I could find from this website

https://rust-unofficial.github.io/patterns/intro.html

Which I think is good for API design in general

Speaking neutraly I should emphasis "unofficial" as it is easy to misread....
and I don't want to make it out as something it is not.

Here is the outline of the convention that is already encoded into bevy

Note that it is common and expected for types to implement both Default and an empty new constructor. new is the constructor convention in Rust, and users expect it to exist, so if it is reasonable for the basic constructor to take no arguments, then it should, even if it is functionally identical to default.

https://rust-lang.github.io/api-guidelines/interoperability.html#types-eagerly-implement-common-traits-c-common-traits

[martinfrances107]

@james7132

Changing this would also likely be advised for all ecosystem crates' documentation too.

Is there a git repo that you are thinking of ,,,, I am not sure. I have a mind to file more PRs

[SludgePhD]

Implementing both Default and a new() constructor is a good idea, but the links you provided do not make a claim about default() being the preferred way to create an instance. That's what the only disagreement here is about.

[mockersf]

I completely disagree with this change. Default has not the same semantic meaning as new, and shouldn't be... the default.

It's recommended by rust to implement it when possible because it makes it easier to work with autoderivation, but it's not a recommendation to use it everywhere.