Skip to content

Commit

Permalink
Remove horrible hack
Browse files Browse the repository at this point in the history
So, as the comment suspected, this did come back to bite me! Fortunately
not in a bad way, as I found this comment pretty quick while figuring
out what was wrong with the new code I wrote. But honestly, I'm
surprised that it lasted this long (almost 10 months!).

To my delight, removing this hack required no changes to any code! I
take that as a sign that we're in a much better position today,
architecture-wise, then we were back then.
  • Loading branch information
hannobraun committed Jul 19, 2023
1 parent a648c57 commit 93fc44e
Showing 1 changed file with 0 additions and 37 deletions.
37 changes: 0 additions & 37 deletions crates/fj-core/src/storage/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,61 +219,24 @@ impl<T> Eq for HandleWrapper<T> {}

impl<T> PartialEq for HandleWrapper<T> {
fn eq(&self, other: &Self) -> bool {
// The purpose of `HandleWrapper` is to provide equality (and other
// traits) for `Handle<T>`s that would otherwise not have them. We use
// `Handle::id` to do this. This means, objects that are not identical
// are not equal.
//
// This is desirable for the most part, but it does become horribly
// inconvenient in test code. Tests, by design, create equal (but not
// identical) objects and compare them against objects produced by the
// code under test. Under such a use case, one would rather ignore any
// objects wrapped by `HandleWrapper`.
//
// And the following bit of code does just that. This might be a
// horrible hack that will comes back to bite us later (I honestly don't
// know), but it is certainly a very economical solution to this
// problem.
if cfg!(test) {
return true;
}

self.0.id().eq(&other.0.id())
}
}

impl<T> Hash for HandleWrapper<T> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
// This piece of code exists to keep this implementation in sync with
// the `PartialEq` implementation. See comment there.
if cfg!(test) {
return;
}

self.0.id().hash(state);
}
}

impl<T> Ord for HandleWrapper<T> {
fn cmp(&self, other: &Self) -> Ordering {
// This piece of code exists to keep this implementation in sync with
// the `PartialEq` implementation. See comment there.
if cfg!(test) {
return Ordering::Equal;
}

self.0.id().cmp(&other.0.id())
}
}

impl<T> PartialOrd for HandleWrapper<T> {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
// This piece of code exists to keep this implementation in sync with
// the `PartialEq` implementation. See comment there.
if cfg!(test) {
return Some(Ordering::Equal);
}

self.0.id().partial_cmp(&other.0.id())
}
}
Expand Down

0 comments on commit 93fc44e

Please sign in to comment.