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

Add orEmpty extensions for immutable collections #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented May 15, 2024

Closes #182.

Copy link

@st-hocnguyen st-hocnguyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about PersistentList.orEmpty: PersistentList, PersistentMap.orEmpty: PersistentMap and PersistentSet.orEmpty: PersistentSet overloads 🙏 ?

@Goooler
Copy link
Contributor Author

Goooler commented Jul 24, 2024

Persistent* extends Immutable*, these extensions work for both of them.

@st-hocnguyen
Copy link

st-hocnguyen commented Jul 24, 2024

Persistent* extends Immutable*, these extensions work for both of them.

But when we need a value of Persistent*, we must do orEmpty().toPersistent*(). I think adding above overloads is useful 🙏 . Just call val nonEmpty: PersistentList<T> = persistentList.orEmpty()

@Goooler
Copy link
Contributor Author

Goooler commented Jul 24, 2024

Seems we can return Persistent* as the value, WHYT?

@hoc081098
Copy link

Seems we can return Persistent* as the value, WHYT?

The change of return type to Persistent is inefficient.

For example, if the list is ImmutableList, but is not PersistentList, orEmpty will convert it to PersistentList, it is inefficient.

The better way is having two overloads, that avoids unnecessary operations.

@Goooler
Copy link
Contributor Author

Goooler commented Jul 24, 2024

I didn't take immutable*Of as the implementation due to the deprecations, they are not suggested to be used.

/**
* Returns a new persistent list of the specified elements.
*/
@Deprecated("Use persistentListOf instead.", ReplaceWith("persistentListOf(*elements)"))
public fun <E> immutableListOf(vararg elements: E): PersistentList<E> = persistentListOf(*elements)
/**
* Returns an empty persistent list.
*/
@Deprecated("Use persistentListOf instead.", ReplaceWith("persistentListOf()"))
public fun <E> immutableListOf(): PersistentList<E> = persistentListOf()
/**
* Returns a new persistent set with the given elements.
*
* Elements of the returned set are iterated in the order they were specified.
*/
@Deprecated("Use persistentSetOf instead.", ReplaceWith("persistentSetOf(*elements)"))
public fun <E> immutableSetOf(vararg elements: E): PersistentSet<E> = persistentSetOf(*elements)
/**
* Returns an empty persistent set.
*/
@Deprecated("Use persistentSetOf instead.", ReplaceWith("persistentSetOf()"))
public fun <E> immutableSetOf(): PersistentSet<E> = persistentSetOf()
/**
* Returns a new persistent set with the given elements.
*
* Order of the elements in the returned set is unspecified.
*/
@Deprecated("Use persistentHashSetOf instead.", ReplaceWith("persistentHashSetOf(*elements)"))
public fun <E> immutableHashSetOf(vararg elements: E): PersistentSet<E> = persistentHashSetOf(*elements)
/**
* Returns a new persistent map with the specified contents, given as a list of pairs
* where the first component is the key and the second is the value.
*
* If multiple pairs have the same key, the resulting map will contain the value from the last of those pairs.
*
* Entries of the map are iterated in the order they were specified.
*/
@Deprecated("Use persistentMapOf instead.", ReplaceWith("persistentMapOf(*pairs)"))
public fun <K, V> immutableMapOf(vararg pairs: Pair<K, V>): PersistentMap<K, V> = persistentMapOf(*pairs)
/**
* Returns a new persistent map with the specified contents, given as a list of pairs
* where the first component is the key and the second is the value.
*
* If multiple pairs have the same key, the resulting map will contain the value from the last of those pairs.
*
* Order of the entries in the returned map is unspecified.
*/
@Deprecated("Use persistentHashMapOf instead.", ReplaceWith("persistentHashMapOf(*pairs)"))
public fun <K, V> immutableHashMapOf(vararg pairs: Pair<K, V>): PersistentMap<K, V> = persistentHashMapOf(*pairs)

@qurbonzoda
Copy link
Contributor

It makes sense to add the same extensions for Persistent counterparts as well.
However, let's postpone adding new extensions until we have a clear solution for handling the issue of stdlib overloads having precedence. See #10 and #181 for details.

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.

No orEmpty function for ImmutableList?
4 participants