Possible issue in unit test on Lens Person exercise

Programming JavaScript
1

Hi,

In the tests for nameLens on the Lens Person exercise, I think there may be an issue.

The test for ensuring immutability expects that the test data person object’s name should not equal “Sara Lakshamanan”.

I think there are several issues causing this test to always pass.

First, it only applies to the “forename” test, where the lens is used to set a new name of “Sara Lakshmanan”. This test doesn’t make sense at all if the “surname” test is not skipped, since in an “incorrect” solution, where the object is mutated, it would then equal “Saravanan Laksh”.

The other issue is that: “Lakshamanan” and “Lakshmanan” are not equal, so even with skipping the “surname” test, this test will always pass.
There is a second typo issue in the streetLens, since “street” and “Street” are not equal.

The first issue doesn’t apply to the other test groups, as they only have one .set test, however I think it would still be better to replace each of these with a test positively checking the specific property of person, or the full expected object:

  • expect(person.name).toStrictEqual(new Name("Saravanan", "Lakshamanan"));
  • expect(person.born.bornAt).toStrictEqual(new Address(100, "Hospital street", "Tamil Nadu", "India"));
  • expect(person.address.street).toStrictEqual("Coder Street");
2

I may just not be understanding the exercise, but get(person) and set(person, value) are defined in the Lens class, but they aren’t necessary since the values provided when using the class replace them.

Also I think there should be more tests, to discourage hard-coding.

Opinion

Also I don’t think the premise of this exercise is entirely valid?
The example given from Haskell does indeed seem very messy, but it isn’t nearly as bad in JavaScript.
Plus why is there a need for an exercise about this functional programming concept in (what I believe to be) an object-oriented language?

3

We can think about functional programming as being a mindset. Some programming languages have many features that specifically support it.

I would suggest that the React framework is built with a functional mindset.

4

@FriendlyUser1 In my humble opinion Javascript, as it is used today in most environments, is very much a functional language. Yes, it supports OOP functionality but with most modern frameworks it feels like that’s really not the focus. It’s very much a matter of choice tho.

AS for the exercise tests, PRs are welcome if you have the time and energy to work on this. You’d also want to discuss with @SleeplessByte (our resident JS track maintainer (sorry for the ping)).

1 Like
5

Adding tests to ensure the immutability (and fixing the typos) is accepted and we’ll take a PR.

6

Opened a PR

7

Merged! You should see the change on the website shortly.

1 Like
8

What happened with this?

They’re not replaced per se, but you’re right. The get property shadows the get method. The method never gets called.

9

The get and set method still need to be removed. Want to PR it?

10

Sure! I’ll get on it.

11
1 Like
12

It has been merged! Thank you very much.

13

I have found another issue with the tests in this exercise. None of the tests for the lens.set method check that the value has actually been set, they just check that the original object hasn’t been mutated. Not a big issue, but kinda fun that this little snippet passes the tests:

const lens = (...path) => ({
	get: o => path.reduce((a, p) => a[p], o),
	set: o => Object.assign({}, o) 
})

export const nameLens = lens('name'), 
             bornAtLens = lens('born', 'bornAt'), 
             streetLens = lens('address', 'street')

Simply shallowcloning the passed object is enough to pass the tests for referential equality, values and nested objects are unchecked.

14

Wow, indeed. The tests check whether the “lens” get value (nameLens(obj)) equals to the “long way round” value (obj.name), but not what that value actually is.

You don’t even have to copy anything, really. You can get away with o => o.

15

Well, if anyone would like to PR this to add a test for the actual value, feel free to ping me for a review.
@themetar @smlsvnssn

16

I think this exercise should be redesigned. I should probably open a new thread, but in short:

I looked into what Lenses actually are. A big property of theirs is that they’re composable. So streetLens should be created with lens('address') + lens('street') (preudocode)

A really simplified explanation on Lenses I’ve found here.

An actual functional explanation here.

So just writing new Lens(p => p.name, (p, v) => { ...o, name: v }) is not using lenses.

Even the Haskell and Scala track don’t require the lens-related functions to be provided to the tests, just the final wrapper that returns the name, address, etc… The “proper” solution is to implement and use lenses within your code, going on that wiki page for Haskell.

The fact that you can solve the exercise without actually interacting with the topic might be a teachable moment in Haskell/Scala - I’m not sure - if the non-lens way is longer and more tedious than the functional way; but in JS a method that accesses a field is rather trivial.

1 Like