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

Redundant condition check in Push method of ArrayStack #6

Open
remoree opened this issue Nov 5, 2023 · 1 comment
Open

Redundant condition check in Push method of ArrayStack #6

remoree opened this issue Nov 5, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@remoree
Copy link

remoree commented Nov 5, 2023

In the current implementation of the Push method in the ArrayStack struct, there is a condition check nextIndex < len(x.slice). However, this condition will never be true due to the way the ArrayStack is initialized and elements are added to it.

The NewArrayStack function initializes the slice with a length of 0, and the Push method only ever increases the length of the slice by appending elements to it. So, the nextIndex will always be equal to the current length of the slice when a new element is being pushed, and the append function will always be used to add the new element to the slice.

Therefore, the condition nextIndex < len(x.slice) is not necessary in the current implementation. The Push method could be simplified as follows:

func (x *ArrayStack[T]) Push(values ...T) {
	for _, value := range values {
		x.slice = append(x.slice, value)
		x.topIndex++
	}
}

This version of the Push method correctly increases the size of the slice as new elements are pushed onto the stack, and updates the top index accordingly.

However, if the goal is to make the Push method more efficient by minimizing memory reallocations, an alternative approach could be used. Here is an example of such an implementation:

func (x *ArrayStack[T]) Push(values ...T) {
	if values == nil || len(values) == 0 {
		return
	}

	// Calculate total space needed for all elements being pushed
	requiredCap := len(x.slice) + len(values)

	// Grow the stack once if necessary
	if requiredCap > cap(x.slice) {
	        // It is assumed that there is a resize method created on the ArrayStack type to increase the capacity here.
		x.resize(requiredCap)
	}

	// Append all the elements
	x.slice = append(x.slice, values...)
}

In this version, the Push method grows the capacity of the slice in a more controlled manner, which can lead to fewer memory reallocations and improved performance.

@songzhibin97
Copy link
Member

Can you help me with a pr to optimize it?

@golanginfrastructure golanginfrastructure added enhancement New feature or request good first issue Good for newcomers labels Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants