Github user FlorianHockmann commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/868#discussion_r189275849
  
    --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs ---
    @@ -0,0 +1,88 @@
    +#region License
    +
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +#endregion
    +
    +using Gremlin.Net.Driver.Exceptions;
    +using System.Collections;
    +using System.Collections.Generic;
    +
    +namespace Gremlin.Net.Driver
    +{
    +    /// <summary>
    +    /// A ResultSet is returned from the submission of a Gremlin script to 
the server and represents the results provided by the server
    +    /// ResultSet includes enumerable data and status attributes.
    +    /// </summary>
    +    /// <typeparam name="T"></typeparam>
    +    public sealed class ResultSet<T> : IReadOnlyCollection<T>
    +    {
    +        /// <summary>
    +        ///  Gets and Sets the read only collection
    +        /// </summary>
    +        public IReadOnlyCollection<T> Data { get; set; }
    +
    +        /// <summary>
    +        /// Gets or Sets the status attributes from the gremlin response
    +        /// </summary>
    +        public Dictionary<string, object> StatusAttributes { get; set; }
    +
    +        /// <summary>Returns an enumerator that iterates through the 
collection.</summary>
    +        /// <returns>An enumerator that can be used to iterate through the 
collection.</returns>
    +        public IEnumerator<T> GetEnumerator()
    +        {
    +            return this.Data.GetEnumerator();
    +        }
    +
    +        /// <summary>Returns an enumerator that iterates through a 
collection.</summary>
    +        /// <returns>An <see cref="T:System.Collections.IEnumerator" /> 
object that can be used to iterate through the collection.</returns>
    +        IEnumerator IEnumerable.GetEnumerator()
    +        {
    +            return this.GetEnumerator();
    +        }
    +
    +        /// <summary>Gets the number of elements in the 
collection.</summary>
    +        /// <returns>The number of elements in the collection. </returns>
    +        public int Count => this.Data.Count;
    +    }
    +
    +    /// <summary>
    +    /// Extension for IReadOnlyCollection 
    +    /// </summary>
    +    public static class ResultSetExtensions
    +    {
    +        /// <summary>
    +        /// Casts a IReadOnlyCollection to ResultSet
    +        /// </summary>
    +        /// <typeparam name="T"></typeparam>
    +        /// <param name="data"></param>
    +        /// <returns></returns>
    +        public static ResultSet<T> AsResultSet<T>(this 
IReadOnlyCollection<T> data)
    --- End diff --
    
    You mean because the methods on `IGremlinClient` return an 
`IReadOnlyCollection` which is therefore what the user is getting?
    I think it would make sense to change that and instead now return a 
`ResultSet` instead. That's in line with Gremlin-Java ([which returns the Java 
equivalent](https://github.com/apache/tinkerpop/blob/master/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java#L272):
 `CompletableFuture<ResultSet>`) and it makes it easier to make the attributes 
available to the user.
    Such a breaking change should also be no big problem as this is targeted 
for 3.4.0. You just have to mention it in the upgrade docs.
    
    Then you can probably remove this method completely.


---

Reply via email to