[GitHub] [avro] yanivru commented on a diff in pull request #1597: AVRO-2211: SchemaBuilder equivalent or other means of schema creation

[GitBox <gi...@apache.org>]

yanivru commented on code in PR #1597:
URL: https://github.com/apache/avro/pull/1597#discussion_r841278792


##########
lang/csharp/src/apache/main/Schema/EnumSchema.cs:
##########
@@ -103,15 +133,46 @@ internal static EnumSchema NewInstance(JToken jtok, PropertyMap props, SchemaNam
                             string doc, string defaultSymbol)
                             : base(Type.Enumeration, name, aliases, props, names, doc)
         {
-            if (null == name.Name) throw new SchemaParseException("name cannot be null for enum schema.");
+            if (null == name.Name) throw new AvroException("name cannot be null for enum schema.");
             this.Symbols = symbols;
             this.symbolMap = symbolMap;
 
             if (null != defaultSymbol && !symbolMap.ContainsKey(defaultSymbol))
-                throw new SchemaParseException($"Default symbol: {defaultSymbol} not found in symbols");
+                throw new AvroException($"Default symbol: {defaultSymbol} not found in symbols");
             Default = defaultSymbol;
         }
 
+        /// <summary>
+        /// Creates symbols map from specified list of symbols.
+        /// Symbol map contains the names of the symbols and their index.
+        /// </summary>
+        /// <param name="symbols">List of symbols</param>
+        /// <returns>Symbol map</returns>
+        /// <exception cref="AvroException">Is thrown if the symbols list contains invalid symbol name or duplicate symbols</exception>
+        private static IDictionary<string, int> CreateSymbolsMap(IEnumerable<string> symbols)
+        {
+            IDictionary<string, int> symbolMap = new Dictionary<string, int>();
+            int i = 0;
+            foreach (var symbol in symbols)
+            {
+                if (ValidateSymbolName(symbol))
+                {
+                    throw new AvroException($"Invalid symbol name: {symbol}");
+                }
+
+                if (symbolMap.ContainsKey(symbol))
+                {
+                    throw new AvroException($"Duplicate symbol: {symbol}");
+                }
+
+                symbolMap[symbol] = i++;
+            }
+
+            return symbolMap;
+        }
+
+        private static bool ValidateSymbolName(string symbol) => string.IsNullOrEmpty(symbol) || !Regex.IsMatch(symbol, "^([A-Za-z_][A-Za-z0-9_]*)$");

Review Comment:
   Validate that returns bool. Don't know what I was thinking. Fixed.
   
   Yeah, Regex is not really good at performance (I don't like the readability either), but since it's a new feature (the parsing code doesn't use this validation) so we are not degrading performance, and since my guess that usually schemas are not created in a tight loop (usually created once and reused), it should be ok. It can be fixed if it does prove to be a problem.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org