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

remove function duplicate the list #957

Closed
mdouaihy opened this issue Dec 11, 2024 · 2 comments · Fixed by #970
Closed

remove function duplicate the list #957

mdouaihy opened this issue Dec 11, 2024 · 2 comments · Fixed by #970
Assignees

Comments

@mdouaihy
Copy link

mdouaihy commented Dec 11, 2024

Describe the bug
The feel expression remove([1,2,3], 0) returns [1,2,3,1,2,3].

This bug is due to the below code

  private def removeFunction = builtinFunction(
    params = List("list", "position"),
    invoke = { case List(ValList(list), ValNumber(position)) =>
      ValList(
        list.take(listIndex(list, position.intValue)) ++ list.drop(listIndex(list, position.intValue + 1)
        )
      )
    }
  )

The goal of this code is to

  • take the first n elements list.take(listIndex(list, position.intValue))
  • drop the n+1 elements list.drop(listIndex(list, position.intValue + 1)
  • concatenate the resuls

The problem is

  • when n is equal to 0, listIndex(list, position.intValue) returns 3 and listIndex(list, position.intValue + 1) is returning 0.
  • The take part is taking the whole list
  • The drop part is taking the whole list
  • The concatenate is duplicating the list.

where ``````

To Reproduce
Execute the feel expression remove([1,2,3], 0)

Expected behavior
Returns null since position 0 is an invalid argument.

Environment

  • FEEL engine version: latest version,
@saig0
Copy link
Member

saig0 commented Dec 12, 2024

@mdouaihy thank you for reporting. 👍

I can confirm the bug in the version 1.19.0. ✅

@saig0 saig0 self-assigned this Jan 16, 2025
@saig0
Copy link
Member

saig0 commented Jan 16, 2025

I adjusted the expected behavior. If the position is 0, the function should return null.

@mdouaihy suggested returning the original list. According to the DMN specification, zero is an invalid argument. Returning null aligns with other functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants