martin-g commented on code in PR #493:
URL: https://github.com/apache/avro-rs/pull/493#discussion_r2871101697


##########
avro/src/schema/name.rs:
##########
@@ -52,72 +57,90 @@ pub type Names = HashMap<Name, Schema>;
 pub type NamesRef<'a> = HashMap<Name, &'a Schema>;
 /// Represents the namespace for Named Schema
 pub type Namespace = Option<String>;
+/// Represents the namespace for Named Schema
+pub type NamespaceRef<'a> = Option<&'a str>;
 
 impl Name {
     /// Create a new `Name`.
     /// Parses the optional `namespace` from the `name` string.
     /// `aliases` will not be defined.
-    pub fn new(name: &str) -> AvroResult<Self> {
-        let (name, namespace) = Name::get_name_and_namespace(name)?;
-        Ok(Self {
-            name,
-            namespace: namespace.filter(|ns| !ns.is_empty()),
-        })
+    pub fn new(name: impl Into<String> + AsRef<str>) -> AvroResult<Self> {
+        Self::new_with_enclosing_namespace(name, None)
     }
 
-    fn get_name_and_namespace(name: &str) -> AvroResult<(String, Namespace)> {
-        validate_schema_name(name)
+    /// Create a new `Name` using the namespace from `enclosing_namespace` if 
absent.
+    pub fn new_with_enclosing_namespace(
+        name: impl Into<String> + AsRef<str>,
+        enclosing_namespace: NamespaceRef,
+    ) -> AvroResult<Self> {
+        // Having both `Into<String>` and `AsRef<str>` allows optimal use in 
both of these cases:
+        // - `name` is a `String`. We can reuse the allocation when 
`enclosing_namespace` is `None`
+        //   or `name` already has a namespace.
+        // - `name` is a `str`. With only `Into<String` we need an extra 
allocation in the case `name`
+        //   doesn't have namespace and `enclosing_namespace` is `Some`. 
Having `AsRef<str>` allows
+        //   skipping that allocation.
+        let name_ref = name.as_ref();
+        let index_of_name = validate_schema_name(name_ref)?;

Review Comment:
   A custom validator may return an index that would lead to out of bounds 
error later. Maybe we should add a check to prevent this here.



-- 
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