viralogic / py-enumerable

A Python module used for interacting with collections of objects using LINQ syntax
MIT License
187 stars 24 forks source link

Add() does not behave as expected on an empty Enumerable #53

Closed kekivelez closed 3 years ago

kekivelez commented 4 years ago

Here is a code snippet to better explain (Note these are run in ipython for quick and dirty tests, hence the 'out' lines in my code blocks below)

test = Enumerable()
out >> []
test.add({'name':'test', 'value':'test'})
out >> [{'name':'test', 'value':'test'}]
test
out >> []
print(test)
out >> []
test.add(42)
out >> [42]
print(test)
out >> []

Seeing current documentation there is nothing that leads me to believe that this behavior is intentional. When I do not create an empty Enumerable first, I get (not quite) the expected results, but it is a result

test = Enumerable({'name':'test', 'value':'test'})
out >> ['name','value']
test.add({'name':'test2', 'value':'test2'})
out >> ['name','value', {'name':'test2','value':'test2'}]
test
out >> ['name','value']
print(test)
out >> ['name','value']
test.add(42)
out >> ['name','value',42]
print(test)
out >> ['name','value']

Also this second case behaves in unexpected ways with the functions. Using first() I would expect {'name':'test', 'value':'test'}, but instead I get 'name'. The first scenario can't even run first() returns an IndexError (probably empty).

I have tried also using concat() in lieu of add() and it produces the exact result (which makes sense as add() calls concat())

I noticed that none of the documentation provides examples on multi-key dictionaries as the collection given to Enumerable and the complex case in the tests folder is a list of dict but only has a single key. The more I mess around with it the more I suspect:

viralogic commented 4 years ago

Thanks for reporting this bug. I have replicated your issue.

viralogic commented 3 years ago

Hi @kekivelez I spent some time on this issue just to investigate further. Here is a unit test that replicates your issue:

def test_issue_53(self):
        test = Enumerable({'name':'test', 'value':'test'})
        test.add({'name':'test2', 'value':'test2'})
        self.assertListEqual(
            ['name','value', {'name':'test2','value':'test2'}],
            test.to_list()
        )
        test.add(42)
        self.assertListEqual(
            ['name','value', {'name':'test2','value':'test2'}, 42],
            test.to_list()
        )

This test fails as expected. However when I change the unit test to this it will pass.

def test_issue_53(self):
        test = Enumerable({'name':'test', 'value':'test'})
        test = test.add({'name':'test2', 'value':'test2'})
        self.assertListEqual(
            ['name','value', {'name':'test2','value':'test2'}],
            test.to_list()
        )
        test = test.add(42)
        self.assertListEqual(
            ['name','value', {'name':'test2','value':'test2'}, 42],
            test.to_list()
        )

So you can see, by overwriting your original test Enumerable instance with each add, the variable is able to retain the new state obtained with each add.

The reason for this behaviour is in the method for concat. This method returns a new ConcatEnumerable instance that retains the new state obtained when concatenating a new iterable with an original Enumerable. However, this method does not mutate the state the of the original Enumerable. In my design for this library, I tried to avoid mutations as much as possible in case users ever want to go back to the original Enumerable state and perform new set of operations.

TLDR Just overwrite your Enumerable instance with the result from the add operation if you want to do inplace mutation of your Enumerable collection.

viralogic commented 3 years ago

I have thought about this some more and these mutator methods (add, concat, prepend, append) should be supported where you don't have to save the result to a new variable or overwrite your own variable. I am re-opening this issue. Apologies for this.

viralogic commented 3 years ago

Latest release is now available that fixes the add issue for you.

pip install -U py-linq