exercism / v3

The work-in-progress project for developing v3 tracks
https://v3.exercism.io
Other
170 stars 162 forks source link

[C#] Implement new Concept Exercise: equality #1016

Closed ErikSchierboom closed 4 years ago

ErikSchierboom commented 4 years ago

This issue describes how to implement the equality concept exercise for the C# track.

Getting started

Please please please read the docs before starting. Posting PRs without reading these docs will be a lot more frustrating for you during the review cycle, and exhaust Exercism's maintainers' time. So, before diving into the implementation, please read up on the following documents:

Please also watch the following video:

Goal

The goal of this exercise is to teach the student the Concept of Equality in C#.

Learning objectives

Out of scope

Concepts

This Concepts Exercise's Concepts are:

Prequisites

This Concept Exercise's prerequisites Concepts are:

Resources to refer to

Hints

After

Representer

This exercise does not require any specific representation logic to be added to the representer.

Analyzer

This exercise does not require any specific analyzer logic to be added to the analyzer.

Implementing

To implement this exercise, please follow these instructions.

Help

If you have any questions while implementing the exercise, please post the questions as comments in this issue.

mikedamay commented 4 years ago

I think dictionaries should be a prerequisite. It is difficult explain GetHashCode() without them and IEquatable<T> needs to reference collections

mikedamay commented 4 years ago

I know we are not dealing with simple types in this exercise, but given the name of the exercise and the second trickiest thing about equality, we should mention in after.md the difficulties with floating point numbers and equality.

mikedamay commented 4 years ago

I think equality of structs works better in structs than here. Overriding Equals and GetHashCode() is enough to take on board for one exercise. In addition, equality is very important to an understanding of structs' nature.

ErikSchierboom commented 4 years ago

I think dictionaries should be a prerequisite. It is difficult explain GetHashCode() without them and IEquatable needs to reference collections

An alternative would be to have sets as a prerequisite. I personally find sets easier to reason about in combination with GetHashCode(). What do you think?

I think equality of structs works better in structs than here. Overriding Equals and GetHashCode() is enough to take on board for one exercise. In addition, equality is very important to an understanding of structs' nature.

Agreed. Good call.

I know we are not dealing with simple types in this exercise, but given the name of the exercise and the second trickiest thing about equality, we should mention in after.md the difficulties with floating point numbers and equality.

Yep, let's do that! We do have a line in the after.md of the floating-point-numbers exercise that mentions this too:

Always be careful when checking the values of floating-point types for equality, as values that can appear to represent the same value could actually be different. See this article for more information. But mentioning it again won't hurt.

mikedamay commented 4 years ago

An alternative would be to have sets as a prerequisite

definitely. With perhaps +1 for dictionary keys in after.md

ErikSchierboom commented 4 years ago

Yep! That could work well.

mikedamay commented 4 years ago

In after.md consider making a reference to the discussion of the difficulties of floating point number equality discussed here.

mikedamay commented 4 years ago

@ErikSchierboom

I am thinking about an identity matching story perhaps including a facial recognition class (as a member of the main identity class). The main identity class would also include an email address. I think this resonates with our demographic and the fact that it's unrealistic (I'm sure fuzzy logic is a big part of identity matching) does not detract too much.

We could have a HashSet of known identities. Do you think there is a case for including the Set concept here? Maybe there is not too much harm in inserting these common library classes (other than list and dictionary) into other exercises as they are really just classes like any other. Students need to know about them but we don't really need to make too much fuss about them. What do you think?

Two Issues:

mikedamay commented 4 years ago

How should we treat equality+inheritance as described in this SO thread.

My vote is for after.md. Let me know what you think.

mikedamay commented 4 years ago

Learning objectives should include

ErikSchierboom commented 4 years ago

My vote is for after.md. Let me know what you think.

Seconded. This is perfect for the after.md document I feel.

ErikSchierboom commented 4 years ago

I am thinking about an identity matching story perhaps including a facial recognition class (as a member of the main identity class). The main identity class would also include an email address. I think this resonates with our demographic and the fact that it's unrealistic (I'm sure fuzzy logic is a big part of identity matching) does not detract too much.

I'm not a huge fan of the story, as facial recognition seems to be a more touchy subject lately. But we could easily tweak this, e.g. by changing it to fingerprint recognition.

We could have a HashSet of known identities. Do you think there is a case for including the Set concept here? Maybe there is not too much harm in inserting these common library classes (other than list and dictionary) into other exercises as they are really just classes like any other. Students need to know about them but we don't really need to make too much fuss about them. What do you think?

I wouldn't mind also including the Set concept here, mainly because they go together well. I do wonder if this wouldn't make the exercise too "big," as equality is already quite a complex concept in itself. What if we would just have an array of known identities? It would not be the idiomatic solution, but we can re-use this exercise's story for the sets exercises and then show how it should have been modelled. We used this approach in the strings/enums exercises too, where strings would represent the log level as a string, but the enums exercise then converted it to an enum. I know @ihid is a big proponent of doing it like this.

Maybe there is not too much harm in inserting these common library classes (other than list and dictionary) into other exercises as they are really just classes like any other.

I'd rather introduce them at a basic level and then expand on them in a separate exercise than just casually use them without introducing them to be honest. See above.

I'd like to shoehorn in ReferenceEquals() to highlight its significance. It is liable to be the most artificial part of the story but I am prepared to live with that.

Many exercises have artificial parts. As long as it slightly makes sense in the context of the story, I think it is fine.

Will an Identity class with a user-defined reference type FacialRecongnitionIdentity member lead to too many representations?

Hard to tell without seeing the code, but I'd say: let's just try create an Example implementation first and see what it would look like.

mikedamay commented 4 years ago

I'm not a huge fan of the story, as facial recognition seems to be a more touchy subject lately. But we could easily tweak this, e.g. by changing it to fingerprint recognition.

Do you not want the story at all or do you just want to avoid discussing facial recognition?

Fingerprint recognition does not work as well (I can concoct something about eye colour and philtrum width) but I have no feel for fingerprints (as it were). What I'm after is a member that is a reference type. A multi-part name would work as well. Agreed?

including the Set concept here...I do wonder if this wouldn't make the exercise too "big,"

Maybe but... The key teaching requirement with sets is to ensure that the student is aware of their existence and their relationship with GetHashCode(). Apart from that, as soon as a student knows about the libraries/classes/methods, they are equipped to understand sets. Sets are the most intuitive software construct.

we can re-use this exercise's story for the sets exercises

I totally understand the benefits of story re-use and am on the lookout for opportunities. I just don't think this is the best case for that approach.

What if we would just have an array of known identities?

Sets have the advantage that they demonstrate the role of GetHashCode().

I'd rather introduce them at a basic level and then expand on them in a separate exercise

I really can't see equality+sets is a no-no but arrays+for+foreach is OK. Why don't I knock something up with sets as a concept and if you really don't like it we can swap it out as a prerequisite?

Please let me know what you think.

ErikSchierboom commented 4 years ago

Do you not want the story at all or do you just want to avoid discussing facial recognition?

The latter. But then again, maybe this is not an issue. As long as we keep the story a positive thing (nothing with surveillance), it could work well. Let's just try to use facial recognition and see how that turns out.

A multi-part name would work as well. Agreed?

Sure, but having thought on it some more (see above), facial recognition could work.

I really can't see equality+sets is a no-no but arrays+for+foreach is OK. Why don't I knock something up with sets as a concept and if you really don't like it we can swap it out as a prerequisite?

Let's be pragmatic and just do this.

iHiD commented 4 years ago

I've not read the discussion properly at all here so feel free to ignore this comment, but "facial recognition" to unlock phones might work as a thing?

mikedamay commented 4 years ago

I have tweaked the story to make this clearer. Logging into a computer network rather than a phone.

mikedamay commented 4 years ago

closed by PR #1691