Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Use structs for element factory #1844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
twsouthwick wants to merge 2 commits into main
base: main
Choose a base branch
Loading
from struct-element-lookup
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,12 @@
using System;
using System.Diagnostics;

namespace DocumentFormat.OpenXml.Framework.Metadata
{
[DebuggerDisplay("{QName,nq}")]
internal sealed class ElementFactory
{
private readonly Func<OpenXmlElement> _factory;

public ElementFactory(in OpenXmlSchemaType type, Func<OpenXmlElement> factory)
{
Type = type;
_factory = factory;
}

public OpenXmlSchemaType Type { get; }
namespace DocumentFormat.OpenXml.Framework.Metadata;

public OpenXmlElement Create() => _factory();

public static ElementFactory Create<T>()
where T : OpenXmlElement, new()
{
var instance = new T();
[DebuggerDisplay("{QName,nq}")]
internal readonly struct ElementFactory(OpenXmlSchemaType type, Func<OpenXmlElement> factory)
{
Comment on lines +10 to +11
Copy link
Preview

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing ElementFactory from a class to a struct introduces value semantics that can lead to unintended copying; please verify that all consumers handle these semantics correctly and that removing the generic Create() method is intentional.

Suggested change
internal readonly struct ElementFactory(OpenXmlSchemaType type, Func<OpenXmlElement> factory)
{
internal class ElementFactory
{
private readonly OpenXmlSchemaType type;
private readonly Func<OpenXmlElement> factory;
public ElementFactory(OpenXmlSchemaType type, Func<OpenXmlElement> factory)
{
this.type = type;
this.factory = factory;
}

Copilot uses AI. Check for mistakes.

public OpenXmlSchemaType Type => type;

return new ElementFactory(instance.Metadata.Type, static () => new T());
}
}
public OpenXmlElement Create() => factory();
}
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,81 +1,49 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Linq;

namespace DocumentFormat.OpenXml.Framework.Metadata
namespace DocumentFormat.OpenXml.Framework.Metadata;

/// <summary>
/// A lookup that identifies properties on an <see cref="OpenXmlElement"/> and caches the schema information
/// from those elements.
/// </summary>
internal class ElementFactoryCollection
{
/// <summary>
/// A lookup that identifies properties on an <see cref="OpenXmlElement"/> and caches the schema information
/// from those elements.
/// </summary>
internal class ElementFactoryCollection
{
public static readonly ElementFactoryCollection Empty = new(Enumerable.Empty<ElementFactory>());
public static readonly ElementFactoryCollection Empty = new([]);

private readonly ElementFactory[] _data;
private readonly List<ElementFactory> _data;

public ElementFactoryCollection(IEnumerable<ElementFactory> lookup)
{
var array = lookup.ToArray();

Array.Sort(array, ElementChildNameComparer.Instance);
public ElementFactoryCollection(List<ElementFactory> lookup)
{
lookup.Sort(ElementChildNameComparer.Instance);
_data = lookup;
}

_data = array;
public OpenXmlElement? Create(in OpenXmlQualifiedName qname)
{
if (_data.Count == 0)
{
return null;
}

public int Count => _data.Length;
// This is on a hot-path and using a dictionary adds substantial time to the lookup. Most child lists are small, so using a sorted
// list to store them with a binary search improves overall performance.
var idx = _data.BinarySearch(new ElementFactory(new(qname, default), null!), ElementChildNameComparer.Instance);

public IEnumerable<ElementFactory> Elements => _data;

public OpenXmlElement? Create(in OpenXmlQualifiedName qname)
if (idx < 0)
{
if (_data.Length == 0)
{
return null;
}

// This is on a hot-path and using a dictionary adds substantial time to the lookup. Most child lists are small, so using a sorted
// list to store them with a binary search improves overall performance.
var idx = Array.BinarySearch(_data, new ElementFactory(new(qname, default), null!), ElementChildNameComparer.Instance);

if (idx < 0)
{
return null;
}

return _data[idx].Create();
return null;
}

private class ElementChildNameComparer : IComparer<ElementFactory>
{
public static IComparer<ElementFactory> Instance { get; } = new ElementChildNameComparer();

private ElementChildNameComparer()
{
}

public int Compare(ElementFactory? x, ElementFactory? y)
{
if (x is null && y is null)
{
return 0;
}

if (x is null)
{
return -1;
}
return _data[idx].Create();
}

if (y is null)
{
return 1;
}
private sealed class ElementChildNameComparer : IComparer<ElementFactory>
{
public static IComparer<ElementFactory> Instance { get; } = new ElementChildNameComparer();

return x.Type.Name.CompareTo(y.Type.Name);
}
}
public int Compare(ElementFactory x, ElementFactory y) => x.Type.Name.CompareTo(y.Type.Name);
}
}
Loading

AltStyle によって変換されたページ (->オリジナル) /