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

New name for assert_lits_equal ? #25

Open
PragTob opened this issue Oct 14, 2021 · 3 comments
Open

New name for assert_lits_equal ? #25

PragTob opened this issue Oct 14, 2021 · 3 comments

Comments

@PragTob
Copy link

PragTob commented Oct 14, 2021

equal implies full equality aka == which seems confusing (and was pointed out as a downside to introducing assertions into our code base).

Not sure what the best name for this is, I introduced a small wrapper called assert_lists_contain_same for now - it's probably not the best for now happy for other variants but I wonder ig this would be a change considered for the library itself?

Wrapper for reference, yes it's just a module for now not a macro:

      import Assertions, except: [assert_lists_equal: 2, assert_lists_equal: 3]
      import AssertionAdjustments
defmodule ssertionAdjustments do
  require Assertions

  def assert_lists_contain_same(list_a, list_b) do
    Assertions.assert_lists_equal(list_a, list_b)
  end

  def assert_lists_contain_same(list_a, list_b, comparison) do
    Assertions.assert_lists_equal(list_a, list_b, comparison)
  end
end
@devonestes
Copy link
Owner

Funny enough, when I think of two lists as "equal," I always think of the contents and not the order, which is why I think this bug comes up so often in so many tests for folks. I (and I think many others) think of == as saying "Make sure these two lists have the same things in them," but what == really says is "Make sure these two lists have the same things in them, and make sure the lists are in the same order," so in my mind == is something more than "equal."

That said, I can totally see how this name might be confusing to many others, though, even though it makes sense to me, so I'm happy to explore a name change here. How about assert_lists_equivalent? Looking at the definition for equivalent I like this third one of corresponding or virtually identical especially in effect or function, so that seems like a potentially good fit. My biggest issue there is that I can never remember how to spell it, but tab complete should make that not so much of a problem 😉

@beligante
Copy link

beligante commented Nov 4, 2021

Funny enough, when I think of two lists as "equal," I always think of the contents and not the order, which is why I think this bug comes up so often in so many tests for folks. I (and I think many others) think of == as saying "Make sure these two lists have the same things in them," but what == really says is "Make sure these two lists have the same things in them, and make sure the lists are in the same order," so in my mind == is something more than "equal."

That said, I can totally see how this name might be confusing to many others, though, even though it makes sense to me, so I'm happy to explore a name change here. How about assert_lists_equivalent? Looking at the definition for equivalent I like this third one of corresponding or virtually identical especially in effect or function, so that seems like a potentially good fit. My biggest issue there is that I can never remember how to spell it, but tab complete should make that not so much of a problem 😉

I share the same thoughts as you @devonestes .
Giving my 2cents on the discussion you've raised, there is another possible solution for your naming where assert_lists_equal could also accept an option to ensure the order. Like this

Assertions.assert_lists_equal(list_a, list_b, ensure_order: true)

This way you could keep the name as it is, and for devs it's cleaner what the default behaviour is

@etehtsea
Copy link

Would assert_lists_equal_unordered be a viable option?

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

4 participants