Adding AddRange to HashSet

[Venando-a]

To ease creation of a chart with multiple data sets, adding the AddRange method to the Dataset HashSet would be beneficial.

[MindSwipe]

Which classes would need to be modified for this? I only saw that LineChartData uses a HashSet

[Venando-a]

Yes, we saw it with LineChartData

[Joelius300]

The HashSet is only used to prevent adding the same Dataset twice. A better solution would be to expose an IReadOnlyList with a standard List as backing field.
This way we can expose only necessary methods like Add, AddRange, Clear. For Add and AddRange we can check if the elements already exist to avoid duplicates.

The steps would be like this (just the concept, this code won't work):

LineChartConfig config = GetConfig();
LineChartDataset dataset = GetDataset();
dataset.AddRange(1, 2, 3, 4); // <-- dataset.Data is readonly
config.Data.Add(dataset); // <-- Data.Datasets is readonly

Can you think of any other methods that would be worth exposing?

[MindSwipe]

I might be ignorant here, but why not keep it a HashSet (so we don't need to worry about duplicates and such) and create an Extension method AddRange for ISet<T>?

[Joelius300]

It's not our responsibility to provide that AddRange function if we keep the HashSet.

Reasons why we should not provide this extension:

1. It's very easy to implement the extension yourself with a foreach.
2. The foreach version isn't very big and can be used inline to work around the lacking AddRange function.
3. This extension doesn't have a direct connection to our library. Yes you can use it to add those datasets but that covers only a very small window you could use that function for.

If we were to use a backing field and only expose our own methods it would be a good idea to also expose AddRange since it's convenient. It would only apply to that scenario and have nothing to do with HashSet.

Since this isn't an important feature and can easily be worked around by implementing an extension yourself, we will not change anything (yet). We will have to be more consistent about using HashSet vs List though.

[SeppPenner]

The extension is quite easy, yes. I have already implemented it and used it as well. We should decide this later when the charts are re-worked.

Just for reference:

using System.Collections.Generic;

namespace ChartJs.Blazor.ChartJS.Extensions
{
    /// <summary>
    /// Extends the <see cref="HashSet{T}"/> functionality.
    /// </summary>
    public static class HashsetExtensions
    {
        /// <summary>
        /// Allows to add a range (e.g. an <see cref="IEnumerable{T}"/> to the <see cref="HashSet{T}"/>.
        /// </summary>
        /// <typeparam name="T">The type of the <see cref="HashSet{T}"/>.</typeparam>
        /// <param name="set">The <see cref="HashSet{T}"/> to add to.</param>
        /// <param name="list">The <see cref="IEnumerable{T}"/> to add to the <see cref="HashSet{T}"/>.</param>
        public static void AddRange<T>(this HashSet<T> set, IEnumerable<T> list)
        {
            foreach(var value in list)
            {
                set.Add(value);
            }
        }
    }
}

[MindSwipe]

Just to note:

I prefer extension methods to be as generic as possible, hence I made one using ISet<T> instead of HashSet<T>.

/// <summary>
/// Allows to add a range of items (e.g an <see cref="IEnumerable{T}"/>) to a <see cref="ISet{T}"/>.
/// </summary>
/// <param name="self">The <see cref="ISet{T}"/> to add to.</param>
/// <param name="toAdd">The range (e.g <see cref="IEnumerable{T}"/>) to add.</param>
/// <typeparam name="T">The type of the <see cref="ISet{T}"/>.</typeparam>
public static void AddRange<T>(this ISet<T> self, IEnumerable<T> toAdd)
{
    foreach (var item in toAdd)
    {
        self.Add(item);
    }
}

[SeppPenner]

@MindSwipe That was something I missed, thanks.