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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to