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

Adding tests for JsonObject with repeated properties names #66

Merged
merged 2 commits into from
Jun 3, 2016

Conversation

awvalenti
Copy link
Contributor

Since allowing properties with repeated names is a deliberate feature, I've added a couple of tests to document that.

@bernardosulzbach
Copy link
Contributor

It seems good.

@bernardosulzbach
Copy link
Contributor

@awvalenti Shouldn't we also have a case for same name, same value? On the current implementation it is quite obvious that it will work just as the first case you added. But I think there is some documentation value for it. We don't only allow for repeated names, we allow even for repeated pairs.

I don't think it is entirely necessary, so feel free to judge if this case gets in or not.

@awvalenti
Copy link
Contributor Author

Done! Please take another look at it.

@bernardosulzbach
Copy link
Contributor

Seems good.

But I was actually talking about

  @Test
  public void keyRepetition_allowsMultipleEntries() {
    object.add("a", "a");
    object.add("a", "a");

    assertEquals(2, object.size());
  }

when I wrote "Shouldn't we also have a case for same name, same value?".

@awvalenti
Copy link
Contributor Author

Oh, ok. I'm not sure if that's necessary, but it's up to you.

@bernardosulzbach
Copy link
Contributor

bernardosulzbach commented Jun 2, 2016 via email

@ralfstx ralfstx merged commit 57a421a into ralfstx:master Jun 3, 2016
@ralfstx
Copy link
Owner

ralfstx commented Jun 3, 2016

Looks good, thank you!

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

Successfully merging this pull request may close these issues.

3 participants