Skip to content

Conversation

@idichekop
Copy link
Collaborator

@idichekop idichekop commented Jul 29, 2025

The RightPadding and LeftPadding functions in the slice package have some issues. Also discussed in issue(#320 ):

  1. RightPadding modifies the given slice in place, while LeftPadding returns a copy of the slice with the padding (or the given slice if the paddingLenght is 0). That is not consistent, and brings problems to the caller as it behaves different depending on the situation.
  2. Both functions cannot deal with paddingLength negative values.

This can be shown by running the following code (or in Play https://go.dev/play/p/y7G86ZQjJBP):

// You can edit this code!
// Click here and start typing.
package main

import (
	"fmt"

	"github.com/duke-git/lancet/v2/slice"
)

func main() {
	nums := []int{1, 2, 3, 4, 5}
	padded := slice.LeftPadding(nums, 0, 3)
	fmt.Println(padded)
	// Output:
	// [0 0 0 1 2 3 4 5]

	// This code has some issues:

	// 1. It returns a new slice if padding lenght is greater than 0 (see above), but the given slice if padding lenght is 0
	paddedWithNothing := slice.LeftPadding(nums, 0, 0)
	paddedWithNothing[0] = 10
	fmt.Println(nums) // Nums modified !
	fmt.Println(paddedWithNothing)

	// 2. Cannot deal with negative padding lenght
	paddedWithNegative := slice.LeftPadding(nums, 0, -3)  \\ <== Fails
	fmt.Println(paddedWithNegative)
}

This PR solves:

a. these two issues,
b. modernizes the code,
c. and includes additional tests for padding with negative length, padding empty slices, and padding nil slices.

The catch!

The proposed code uses the standard lib "slices" package for creating the padding suffix/prefix and concatenating the slices. The dependency to the standard lib is already present in the lancet module, BUT was not present in the slice package.

I propose we accept this dependency. Since Go1.21 the "slices" package was made official and has many potentials to modernize parts of the current lancet code base. It can be used also to improve other functions (later).

@duke-git , If you think this dependency is not desired, I can re-write the code to mimic it. My point is that some of the functionality on the std lib can be used (also on other functions) to make it safer, more consistent, and similar behavior on corner cases (e.g. dealing with empty slices). The dependency of lancet to the std lib is already accepted and documented.

@idichekop
Copy link
Collaborator Author

Do not understand exactly why the check does not work. The build complaints when importing a std lib package. Locally all tests pass. Must the CI/CD include some go mod <?> to udpdate? Strange.

@duke-git
Copy link
Owner

Do not understand exactly why the check does not work. The build complaints when importing a std lib package. Locally all tests pass. Must the CI/CD include some go mod <?> to udpdate? Strange.

@idichekop, The check failed because the go version in github action was specified to 1.20, which doesn't contain slices package. see screenshot below.
image

image

@duke-git
Copy link
Owner

The RightPadding and LeftPadding functions in the slice package have some issues. Also discussed in issue(#320 ):

  1. RightPadding modifies the given slice in place, while LeftPadding returns a copy of the slice with the padding (or the given slice if the paddingLenght is 0). That is not consistent, and brings problems to the caller as it behaves different depending on the situation.
  2. Both functions cannot deal with paddingLength negative values.

This can be shown by running the following code (or in Play https://go.dev/play/p/y7G86ZQjJBP):

// You can edit this code!
// Click here and start typing.
package main

import (
	"fmt"

	"github.com/duke-git/lancet/v2/slice"
)

func main() {
	nums := []int{1, 2, 3, 4, 5}
	padded := slice.LeftPadding(nums, 0, 3)
	fmt.Println(padded)
	// Output:
	// [0 0 0 1 2 3 4 5]

	// This code has some issues:

	// 1. It returns a new slice if padding lenght is greater than 0 (see above), but the given slice if padding lenght is 0
	paddedWithNothing := slice.LeftPadding(nums, 0, 0)
	paddedWithNothing[0] = 10
	fmt.Println(nums) // Nums modified !
	fmt.Println(paddedWithNothing)

	// 2. Cannot deal with negative padding lenght
	paddedWithNegative := slice.LeftPadding(nums, 0, -3)  \\ <== Fails
	fmt.Println(paddedWithNegative)
}

This PR solves:

a. these two issues, b. modernizes the code, c. and includes additional tests for padding with negative length, padding empty slices, and padding nil slices.

The catch!

The proposed code uses the standard lib "slices" package for creating the padding suffix/prefix and concatenating the slices. The dependency to the standard lib is already present in the lancet module, BUT was not present in the slice package.

I propose we accept this dependency. Since Go1.21 the "slices" package was made official and has many potentials to modernize parts of the current lancet code base. It can be used also to improve other functions (later).

@duke-git , If you think this dependency is not desired, I can re-write the code to mimic it. My point is that some of the functionality on the std lib can be used (also on other functions) to make it safer, more consistent, and similar behavior on corner cases (e.g. dealing with empty slices). The dependency of lancet to the std lib is already accepted and documented.

Hi, @idichekop. Thanks for your proposal and consideration regarding code modernization. Leveraging the standard library's slices package would indeed enhance code safety, consistency, and lay a solid foundation for optimizing other functions in the future.

However, introducing this dependency conflicts with Lancet's current compatibility commitment. As you're aware, Lancet currently supports Go 1.18 and above, while the slices package was officially introduced in Go 1.21. Directly depending on it would break compatibility for users still on Go 1.18-1.20, which goes against our existing support policy.

To address this, I'd suggest a compromise: instead of relying on the slices package directly, we could implement equivalent functionality internally for now to maintain compatibility with older Go versions. once Lancet raises its minimum supported Go version to 1.21, we can plan to migrate to the standard slices package.

@idichekop
Copy link
Collaborator Author

The RightPadding and LeftPadding functions in the slice package have some issues. Also discussed in issue(#320 ):

  1. RightPadding modifies the given slice in place, while LeftPadding returns a copy of the slice with the padding (or the given slice if the paddingLenght is 0). That is not consistent, and brings problems to the caller as it behaves different depending on the situation.
  2. Both functions cannot deal with paddingLength negative values.

This can be shown by running the following code (or in Play https://go.dev/play/p/y7G86ZQjJBP):

// You can edit this code!
// Click here and start typing.
package main

import (
	"fmt"

	"github.com/duke-git/lancet/v2/slice"
)

func main() {
	nums := []int{1, 2, 3, 4, 5}
	padded := slice.LeftPadding(nums, 0, 3)
	fmt.Println(padded)
	// Output:
	// [0 0 0 1 2 3 4 5]

	// This code has some issues:

	// 1. It returns a new slice if padding lenght is greater than 0 (see above), but the given slice if padding lenght is 0
	paddedWithNothing := slice.LeftPadding(nums, 0, 0)
	paddedWithNothing[0] = 10
	fmt.Println(nums) // Nums modified !
	fmt.Println(paddedWithNothing)

	// 2. Cannot deal with negative padding lenght
	paddedWithNegative := slice.LeftPadding(nums, 0, -3)  \\ <== Fails
	fmt.Println(paddedWithNegative)
}

This PR solves:
a. these two issues, b. modernizes the code, c. and includes additional tests for padding with negative length, padding empty slices, and padding nil slices.

The catch!

The proposed code uses the standard lib "slices" package for creating the padding suffix/prefix and concatenating the slices. The dependency to the standard lib is already present in the lancet module, BUT was not present in the slice package.
I propose we accept this dependency. Since Go1.21 the "slices" package was made official and has many potentials to modernize parts of the current lancet code base. It can be used also to improve other functions (later).
@duke-git , If you think this dependency is not desired, I can re-write the code to mimic it. My point is that some of the functionality on the std lib can be used (also on other functions) to make it safer, more consistent, and similar behavior on corner cases (e.g. dealing with empty slices). The dependency of lancet to the std lib is already accepted and documented.

Hi, @idichekop. Thanks for your proposal and consideration regarding code modernization. Leveraging the standard library's slices package would indeed enhance code safety, consistency, and lay a solid foundation for optimizing other functions in the future.

However, introducing this dependency conflicts with Lancet's current compatibility commitment. As you're aware, Lancet currently supports Go 1.18 and above, while the slices package was officially introduced in Go 1.21. Directly depending on it would break compatibility for users still on Go 1.18-1.20, which goes against our existing support policy.

To address this, I'd suggest a compromise: instead of relying on the slices package directly, we could implement equivalent functionality internally for now to maintain compatibility with older Go versions. once Lancet raises its minimum supported Go version to 1.21, we can plan to migrate to the standard slices package.

That is a fair explanation, Duke! I was not aware of this commitment. I will do as you say. I will submit another version soon.

@duke-git
Copy link
Owner

duke-git commented Aug 1, 2025

The RightPadding and LeftPadding functions in the slice package have some issues. Also discussed in issue(#320 ):

  1. RightPadding modifies the given slice in place, while LeftPadding returns a copy of the slice with the padding (or the given slice if the paddingLenght is 0). That is not consistent, and brings problems to the caller as it behaves different depending on the situation.
  2. Both functions cannot deal with paddingLength negative values.

This can be shown by running the following code (or in Play https://go.dev/play/p/y7G86ZQjJBP):

// You can edit this code!
// Click here and start typing.
package main

import (
	"fmt"

	"github.com/duke-git/lancet/v2/slice"
)

func main() {
	nums := []int{1, 2, 3, 4, 5}
	padded := slice.LeftPadding(nums, 0, 3)
	fmt.Println(padded)
	// Output:
	// [0 0 0 1 2 3 4 5]

	// This code has some issues:

	// 1. It returns a new slice if padding lenght is greater than 0 (see above), but the given slice if padding lenght is 0
	paddedWithNothing := slice.LeftPadding(nums, 0, 0)
	paddedWithNothing[0] = 10
	fmt.Println(nums) // Nums modified !
	fmt.Println(paddedWithNothing)

	// 2. Cannot deal with negative padding lenght
	paddedWithNegative := slice.LeftPadding(nums, 0, -3)  \\ <== Fails
	fmt.Println(paddedWithNegative)
}

This PR solves:
a. these two issues, b. modernizes the code, c. and includes additional tests for padding with negative length, padding empty slices, and padding nil slices.

The catch!

The proposed code uses the standard lib "slices" package for creating the padding suffix/prefix and concatenating the slices. The dependency to the standard lib is already present in the lancet module, BUT was not present in the slice package.
I propose we accept this dependency. Since Go1.21 the "slices" package was made official and has many potentials to modernize parts of the current lancet code base. It can be used also to improve other functions (later).
@duke-git , If you think this dependency is not desired, I can re-write the code to mimic it. My point is that some of the functionality on the std lib can be used (also on other functions) to make it safer, more consistent, and similar behavior on corner cases (e.g. dealing with empty slices). The dependency of lancet to the std lib is already accepted and documented.

Hi, @idichekop. Thanks for your proposal and consideration regarding code modernization. Leveraging the standard library's slices package would indeed enhance code safety, consistency, and lay a solid foundation for optimizing other functions in the future.
However, introducing this dependency conflicts with Lancet's current compatibility commitment. As you're aware, Lancet currently supports Go 1.18 and above, while the slices package was officially introduced in Go 1.21. Directly depending on it would break compatibility for users still on Go 1.18-1.20, which goes against our existing support policy.
To address this, I'd suggest a compromise: instead of relying on the slices package directly, we could implement equivalent functionality internally for now to maintain compatibility with older Go versions. once Lancet raises its minimum supported Go version to 1.21, we can plan to migrate to the standard slices package.

That is a fair explanation, Duke! I was not aware of this commitment. I will do as you say. I will submit another version soon.

Thanks for your understanding, Just send over the revised version when you’re ready. Let me know if you run into any issues.

@idichekop
Copy link
Collaborator Author

New proposal submitted.

As suggested, I implemented the functionalities inspired by the standard lib. I placed them in the internal package of slice because providing them as public in the package might imply some change in the interface (requiring proper versioning, etc...). It would also characterize new features, instead of just a refactor.

As I am not yet sure how @duke-git deals with new features and versioning yet, I propose we keep them for the moment as internal. Later, we can transfer the functions and make them accessible if so desired.

Cheers,

Idichekop

@idichekop idichekop self-assigned this Aug 3, 2025
@duke-git
Copy link
Owner

duke-git commented Aug 4, 2025

New proposal submitted.

As suggested, I implemented the functionalities inspired by the standard lib. I placed them in the internal package of slice because providing them as public in the package might imply some change in the interface (requiring proper versioning, etc...). It would also characterize new features, instead of just a refactor.

As I am not yet sure how @duke-git deals with new features and versioning yet, I propose we keep them for the moment as internal. Later, we can transfer the functions and make them accessible if so desired.

Cheers,

Idichekop

@idichekop, cool. Lancet's release strategy is a variant of Semantic Versioning, defined as follows:

  1. Major: Incremented when multiple features are redesigned and several breaking changes are introduced.
  2. Minor: Incremented when the PATCH version reaches 10 (e.g., 2.2.10 → 2.3.0).
  3. Patch: Incremented for newly added features and backward-compatible bug fixes, as long as backward compatibility is maintained.

We can revise the second rule to follow Semantic Versioning: increment the MINOR version when new features are added in a backward-compatible manner.

Lancet typically release updates for new features every 2 to 3 months. For critical bug fixes or security-related patches, however, releases will be made immediately. Since these are inspired by standard library patterns and could eventually add value as public utilities, I agree with your proposal. Let’s keep them internal for now but flag them for now, and release in v2.3.8 later.

@duke-git duke-git merged commit 6f703fe into duke-git:rc Aug 4, 2025
1 check passed
@idichekop idichekop deleted the fix-package-slice-RightPadding-and-LeftPadding branch August 12, 2025 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants