Repository: paritytech/substrate
File: frame/nfts/src/impl_nonfungibles.rs (Rust)

Code before:

		let collection = NextCollectionId::<T, I>::get()
			.or(T::CollectionId::initial_value())
			.ok_or(Error::<T, I>::UnknownCollection)?;

		Self::do_create_collection(
			collection,
			who.clone(),
			admin.clone(),
			*config,
			T::CollectionDeposit::get(),
			Event::Created { collection, creator: who.clone(), owner: admin.clone() },
		)?;

		Self::set_next_collection_id(collection);

		Ok(collection)
	}

	/// Create a collection of nonfungible items with `collection` Id to be owned by `who` and
	/// managed by `admin`. Should be only used for applications that do not have an incremental
	/// order for the collection IDs and is a replacement for the auto id creation.
	///
	///
	/// SAFETY: This function can break the pallet if it is used in combination with the auto
	/// increment functionality, as it can claim a value in the ID sequence.
	fn create_collection_with_id(
		collection: T::CollectionId,
		who: &T::AccountId,
		admin: &T::AccountId,
		config: &CollectionConfigFor<T, I>,
	) -> Result<(), DispatchError> {
		// DepositRequired can be disabled by calling the force_create() only
		ensure!(
			!config.has_disabled_setting(CollectionSetting::DepositRequired),
			Error::<T, I>::WrongSetting
		);

		Self::do_create_collection(
			collection,
			who.clone(),
			admin.clone(),
			*config,
			T::CollectionDeposit::get(),
			Event::Created { collection, creator: who.clone(), owner: admin.clone() },
		)
	}
}

impl<T: Config<I>, I: 'static> Destroy<<T as SystemConfig>::AccountId> for Pallet<T, I> {
	type DestroyWitness = DestroyWitness;

Review comment (suggestion):
```suggestion

	/// Create a collection of non-fungible items with a `collection` ID to be owned by `who` and
	/// managed by `admin`. This is a replacement for the auto ID creation (handled by the `create_collection` function) and should only be used by applications that do not require an incremental order of collection IDs.
	///
	///
	/// SAFETY: This function will break the pallet if it is used in combination with automatic ID
	/// incrementation as it can claim a value that already exists.
```

Code after:

		let collection = NextCollectionId::<T, I>::get()
			.or(T::CollectionId::initial_value())
			.ok_or(Error::<T, I>::UnknownCollection)?;

		Self::do_create_collection(
			collection,
			who.clone(),
			admin.clone(),
			*config,
			T::CollectionDeposit::get(),
			Event::Created { collection, creator: who.clone(), owner: admin.clone() },
		)?;

		Self::set_next_collection_id(collection);

		Ok(collection)
	}

	/// Create a collection of nonfungible items with `collection` Id to be owned by `who` and
	/// managed by `admin`. Should be only used for applications that do not have an
	/// incremental order for the collection IDs and is a replacement for the auto id creation.
	///
	///
	/// SAFETY: This function can break the pallet if it is used in combination with the auto
	/// increment functionality, as it can claim a value in the ID sequence.
	fn create_collection_with_id(
		collection: T::CollectionId,
		who: &T::AccountId,
		admin: &T::AccountId,
		config: &CollectionConfigFor<T, I>,
	) -> Result<(), DispatchError> {
		// DepositRequired can be disabled by calling the force_create() only
		ensure!(
			!config.has_disabled_setting(CollectionSetting::DepositRequired),
			Error::<T, I>::WrongSetting
		);

		Self::do_create_collection(
			collection,
			who.clone(),
			admin.clone(),
			*config,
			T::CollectionDeposit::get(),
			Event::Created { collection, creator: who.clone(), owner: admin.clone() },
		)
	}
}

impl<T: Config<I>, I: 'static> Destroy<<T as SystemConfig>::AccountId> for Pallet<T, I> {
	type DestroyWitness = DestroyWitness;