v2, к #1424: исправлено получение свойств для Структур #1503
Conversation
WalkthroughThis pull request refactors the constructors in FixedStructureImpl and StructureImpl by replacing multiple conditional checks with concise switch expressions. FixedStructureImpl now includes a new method GetPropCount(), while StructureImpl introduces a new method GetPropertyInfo() and updates its property iteration and Insert logic. In addition, two new exported test procedures in tests/reflector.os validate the correct creation and introspection of both standard and fixed structure properties. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Struct as FixedStructureImpl/StructureImpl
Client->>Struct: Call Constructor(rawArgument)
Struct->>Struct: Evaluate rawArgument via switch expression
alt Valid type (null, BslStringValue, StructureImpl)
Struct-->>Client: Return new structure instance
else Unsupported type
Struct-->>Client: Throw RuntimeException
end
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/reflector.os (2)
935-941: Consider enhancing test coverageWhile the test verifies basic functionality, consider adding assertions for:
- Property types
- Values of both fields
- Property access rights
Процедура ТестДолжен_ПроверитьПолучитьТаблицуСвойствДляСтруктуры() Экспорт Рефлектор = Новый Рефлектор; Структура = Новый Структура("Поле1, Поле2", -1, -2); Таблица = Рефлектор.ПолучитьТаблицуСвойств(Структура); юТест.ПроверитьРавенство(2, Таблица.Количество()); юТест.ПроверитьНеРавенство(Неопределено, Таблица.Найти("Поле1")); + юТест.ПроверитьНеРавенство(Неопределено, Таблица.Найти("Поле2")); + + Поле1 = Таблица.Найти("Поле1"); + юТест.ПроверитьРавенство(Поле1.Значение, -1); + юТест.ПроверитьИстину(Поле1.Чтение); + юТест.ПроверитьИстину(Поле1.Запись); + + Поле2 = Таблица.Найти("Поле2"); + юТест.ПроверитьРавенство(Поле2.Значение, -2); + юТест.ПроверитьИстину(Поле2.Чтение); + юТест.ПроверитьИстину(Поле2.Запись); КонецПроцедуры
943-949: Consider enhancing test coverage for FixedStructureWhile the test verifies basic functionality, consider adding assertions specific to FixedStructure's read-only nature and property values.
Процедура ТестДолжен_ПроверитьПолучитьТаблицуСвойствДляФиксированнойСтруктуры() Экспорт Рефлектор = Новый Рефлектор; ФиксСтруктура = Новый ФиксированнаяСтруктура("Поле1, Поле2", -1, -2); Таблица = Рефлектор.ПолучитьТаблицуСвойств(ФиксСтруктура); юТест.ПроверитьРавенство(2, Таблица.Количество()); юТест.ПроверитьНеРавенство(Неопределено, Таблица.Найти("Поле1")); + юТест.ПроверитьНеРавенство(Неопределено, Таблица.Найти("Поле2")); + + Поле1 = Таблица.Найти("Поле1"); + юТест.ПроверитьРавенство(Поле1.Значение, -1); + юТест.ПроверитьИстину(Поле1.Чтение); + юТест.ПроверитьЛожь(Поле1.Запись, "FixedStructure properties should be read-only"); + + Поле2 = Таблица.Найти("Поле2"); + юТест.ПроверитьРавенство(Поле2.Значение, -2); + юТест.ПроверитьИстину(Поле2.Чтение); + юТест.ПроверитьЛожь(Поле2.Запись, "FixedStructure properties should be read-only"); КонецПроцедуры
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/OneScript.StandardLibrary/Collections/FixedStructureImpl.cs(2 hunks)src/OneScript.StandardLibrary/Collections/StructureImpl.cs(5 hunks)tests/reflector.os(2 hunks)
🔇 Additional comments (5)
src/OneScript.StandardLibrary/Collections/FixedStructureImpl.cs (2)
60-63: LGTM! Clean implementation of GetPropCount()The method correctly delegates to the underlying structure instance, maintaining consistency with the class's design pattern.
169-179: LGTM! Clean switch expression refactorThe switch expression makes the constructor logic more concise and readable while maintaining all the necessary type checks and error handling.
src/OneScript.StandardLibrary/Collections/StructureImpl.cs (3)
29-41: LGTM! Improved constructor readabilityThe refactored constructor uses cleaner loop syntax while maintaining the same functionality.
135-144: LGTM! Well-structured property info implementationThe GetPropertyInfo method provides comprehensive property metadata using a clean builder pattern implementation.
254-264: LGTM! Consistent switch expression refactorThe constructor refactoring matches the style used in FixedStructureImpl, providing consistency across the codebase.
Summary by CodeRabbit
New Features
Refactor
Tests