[WIP] Add complex numbers exercise - Part I

[robkeim]

This is for #342

This is Part I because I didn't do the exponent portion yet, I wanted to get feedback first.

A couple of questions:

1. How do you feel about having the complex numbers being sent as raw Tuples? If I were creating a whole program I'd want a ComplexNumber class which I created in my solution, but I'm not sure that we want to force students to go in that direction if they don't want to so Tuples leaves implementation choices open.
2. What do you think about the difficulty for this exercise? While the math concepts are pretty complex the actual solution is fairly easy and can be solved even if you don't really understand the math behind it.

@ErikSchierboom the generator for this turned out to be pretty easy after all :)

[ErikSchierboom]

Well, I've given it some thought, and I do think we should be using a separate ComplexNumber class (or struct). Here's why:

• Tuples don't give the user any indication on what its member names are (although with C# 7 you can), making it harder for the user to understand what is happening
• Tuples are used in several other exercises and thus we don't need this exercise to explain tuples
• I think having a class/struct feels more natural for this problem, as complex numbers are almost always encoded as real classes
• The exercise is of difficulty 6, which means that we can expect a certain level of proficiency

[jpreese]

I would agree that a ComplexNumber class is the way to go. There already exist similar C# classes to achieve a similar concept (e.g. BigInteger). I also believe creating a new class is actually a little easier and more natural for students, and that a Tuple would make things harder in the end. I know I've written more classes than I have Tuples.

[robkeim]

@ErikSchierboom @jpreese thanks for the great feedback. I've updated the exercise with a ComplexNumber struct, so you guys can let me know what you think :)

[ErikSchierboom]

It is starting to look great! Two remarks.

[ErikSchierboom]

We don't need this case anymore, do we?

[robkeim]

Correct I can remove this now

[ErikSchierboom]

I was wondering of the static class approach makes sense. Shouldn't we be using instance methods?

[robkeim]

@ErikSchierboom I'm fine with that approach which will end up with using having pretty much the same solution as they do in the Java track:
https://github.com/exercism/java/blob/master/exercises/complex-numbers/src/example/java/ComplexNumber.java

The getReal and getImaginary methods are trivially to implement, but that's definitely not the main complexity of the question.

Another thing is once I implement part II, I'll have to convert the integers to be double to represent non-whole numbers with pi and e.

[ErikSchierboom]

Good points! Good luck with pt. II

[ErikSchierboom]

@robkeim The build has failed. Any idea why?

[jpreese]

@ErikSchierboom @robkeim It looks like ComplexNumbers wasn't added to the "All" solutions file.

[robkeim]

@ErikSchierboom @jpreese I'm making progress on this and it now includes Part II as well :) Now there are two issues left:

1. The build issues -> My VisualStudio made a mess of the sln files when I tried to add this project, how have you been creating the references?
2. How should I handle the floating point precision issue (causing three tests to fail currently)? I can either override the .Equals which is what I would do if I were implementing the class, but that puts more code into the student solution, or I could create a custom comparison for ComplexNumber which is then dependent on accessing Real and Imaginary so it won't work if the students haven't implemented those yet. Thoughts?

[jpreese]

1. This is a known issue. I don't think it should be problematic to just update the GUIDs. See the discussion here Visual Studio and Visual Studio for Mac overwrite each other's changes in the solution dotnet/project-system#1821

We also removed the Default from the build, so I don't think we even need this solution anymore. But we might still need it for legacy reasons that I'm unaware of so @ErikSchierboom can confirm or deny that.

2. I can't pull in your branch right now, but which tests are failing for you? I didn't really think the solution in the Example file mattered much, so however you solve it that makes all of the tests pass would seem OK to me.

[robkeim]

@jpreese thanks for the linked issue about the GUIDs, I'll just let Visual Studio do it's thing then :)

Regarding the failing test cases it's these two that are failing:

• Conjugate_a_purely_real_number
• Eulers_identity_formula

The reason for the failures is due to double precision so the answers are 1e-10 or less off. Even if I just returned a hard-coded solution with the current implementation no students are going to be able to solve it successfully and pass all of the tests without hard-coding themselves. The Java track had the same problem and they added a tolerance when comparing equality:
https://github.com/exercism/java/blob/master/exercises/complex-numbers/src/test/java/ComplexNumberTest.java

[ErikSchierboom]

I think I know an easy solution. IIRC there is an Assert.Equal overload that takes a third parameter: the precision to use when comparing equality.

[robkeim]

@ErikSchierboom yes that method exists:
https://msdn.microsoft.com/en-us/library/ms243458.aspx

However, that means the tests are going to have to call Real and Imaginary in order to get those values to them assert equality which means both of those methods have to be implemented first before any of the other methods can be tested. That shouldn't be problematic but it's not the same order that the canonical data has the exercises in.

[ErikSchierboom]

Hmmm, I don't have a great solution then

[ErikSchierboom]

By the way, I'd like to keep the Default solution for now, as I have plans that require it :) I'll create an issue for it.

[robkeim]

@ErikSchierboom I think that's the first time I've seen you not have a solution to a problem I'm asking... guess there's a first time for everything :)

I've gone ahead and created a custom comparison that takes into account the precision and now the tests are green! You can go ahead and review @ErikSchierboom and @jpreese.

[ErikSchierboom]

@robkeim Don't hold it against me! ;) But I miss the comparer, isn't it just the precision overload of Assert.Equal that you are using?

Looking great BTW!

[robkeim]

@ErikSchierboom I won't hold it against you don't worry :)

Yes I decided to go down the route of the Assert.Equal overload to not have to add the comparer into the ComplexNumber class itself where the rest of the solution is being written. That prevents anyone from potentially being confused and think that they can/should also update the comparer.