Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better object duplicate error #708

Open
antoine-de opened this issue Sep 24, 2020 · 4 comments
Open

better object duplicate error #708

antoine-de opened this issue Sep 24, 2020 · 4 comments

Comments

@antoine-de
Copy link
Contributor

Hi,
When using gtfs2netexfr we have some errors like "identifier 8593027 already exists" (as the last log of a netex generation).
To help the diagnostic, it would be helpful to know the collection having duplicates and maybe explain at a meta level (in the main ?) that the problem blocks the netex generation.

@datanel
Copy link
Member

datanel commented Sep 24, 2020

I agree this would be helpful

@woshilapin
Copy link
Contributor

This message comes directly from https://github.com/CanalTP/typed_index_collection/blob/v1.1.0/src/error.rs#L7. The type of object is not known inside this generic library (we know the struct but can't really display anything). I see a few options on how to handle this problem (there might be more).

Fix in typed_index_collection

Doing this probably means to add an additional constraint on T like T: Display or T: Debug in order to be able to display the struct of the type of object.

I'm not really in favor of that solution because:

  • adding a constraint only for reporting feels wrong (and might reduce the capabilities of typed_index_collection)
  • implementation of Display or Debug is likely to not show only the type but the entire object, we don't have control on how users will implement these traits.

Introduce a Trait

To fix the previous solution, we could introduce a specific trait typed_index_collection::ObjectType for example. It's still adding a constraint to T but at least, we do not depend on implementation of Debug or Display that might not be to our taste.

Handle errors in transit_model

Most likely, we're gonna be able to create better error in transit_model. With thiserror for example, we could do something like this.

enum ObjectType {
	Network,
	Line,
	Etc,
} // Implement Display on that `enum`

#[derive(Error)]
enum TMError {
	#[error("identifier {1} of type {0} already exists")]
	IdentifierAlreadyExists(ObjectType, String, #[source] typed_index_collection::Error)
	#[error("other error")]
	OtherError,
}

Since we have plans to migrate transit_model from failure to a more modern framework (thiserror has been discussed for example), maybe this issue can be adressed once the migration happened? Note that we plan to do it but we didn't scheduled it yet and have no idea when it could happen.

@antoine-de
Copy link
Contributor Author

will be closed by hove-io/typed_index_collection#11 no?

@woshilapin
Copy link
Contributor

I don't think so since this piece of code is not going to output the type of object. However, we might be a step closer to achieve that.

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

No branches or pull requests

3 participants