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

RFC: Moving fmt::Display from Predicates to a separate method #41

Closed
XAMPPRocky opened this issue May 29, 2018 · 2 comments
Closed

RFC: Moving fmt::Display from Predicates to a separate method #41

XAMPPRocky opened this issue May 29, 2018 · 2 comments
Labels
breaking-change question Uncertainty is involved

Comments

@XAMPPRocky
Copy link

XAMPPRocky commented May 29, 2018

With adding a way to show predicates with the item being evaluated keeping the current non-evaluated as the Display implementation is a bit confusing as two completely distinct ways of creating a humane readable string. I propose that the fmt::Display implementation be removed and be moved to a seperate method so that's easy to understand that when you want to display a human readable version of a predicate you call either of the methods.

Current

Evaluated

let predicate_fn = predicate::ne(5).not().and(predicate::ge(5));
let (result, output) = predicate_fn.stringify(&5);
println!("{}", output);

Non-evaluated

let predicate_fn = predicate::ne(5).not().and(predicate::ge(5));
println!("{}", predicate_fn);

Proposed

Evaluated

let predicate_fn = predicate::ne(5).not().and(predicate::ge(5));
let (result, output) = predicate_fn.stringify_with_item(&5);
println!("{}", output);

Non-evaluated

let predicate_fn = predicate::ne(5).not().and(predicate::ge(5));
let (result, output) = predicate_fn.stringify();
println!("{}", output);
@epage epage added question Uncertainty is involved breaking-change labels May 29, 2018
@epage
Copy link
Contributor

epage commented Jul 21, 2018

Before we answer this, we need to answer whether to have an "evaluated" version

Benefits of "evaluated"

Benefits of no "evaluated" version

  • Do not need to switch from Predicate<Item> to Predicate<Item: Debug>
  • Overly long Display can hide information. An expression can be nice and short and the parameters / by-products can contain the long-form information (see A way to display *why* a predicate failed #60). We can work around it but its less of a whack-a-mole game without it.

In theory these points are also buried in the notes of #7. There might be more stuff there.

@epage
Copy link
Contributor

epage commented Aug 2, 2018

I think with the current design, it is safe to stay with how things are.

@epage epage closed this as completed Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change question Uncertainty is involved
Projects
None yet
Development

No branches or pull requests

2 participants