Реализация пользовательских итераторов #1385
Conversation
WalkthroughОбновления включают улучшение поддержки многоязычности, уточнение обработки директив сценарного движка и расширение контекстов итерации. Был добавлен новый метод для проверки существования имени в двуязычных контекстах, дублирование регистрации службы и создание контекста итерации пользователя на основе атрибутов модуля. Кроме того, были внедрены предопределенные аннотации для сравнения имен без учета регистра и оболочка для сценарных перечислителей, что указывает на фокус на расширяемости и интернационализации в сценарном движке. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Analysis Details1 Issue
Coverage and Duplications
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (8)
- src/OneScript.Language/Localization/BilingualString.cs (2 hunks)
- src/ScriptEngine/Hosting/EngineBuilderExtensions.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/AttachedScriptsFactory.cs (2 hunks)
- src/ScriptEngine/Machine/Contexts/BslPredefinedAnnotation.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/IterableDirectiveProvider.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/PredefinedDirectiveHandler.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/ScriptedEnumeratorWrapper.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/UserIterableContextInstance.cs (1 hunks)
Additional comments: 13
src/ScriptEngine/Machine/Contexts/IterableDirectiveProvider.cs (1)
- 12-20: The
IterableDirectiveProviderclass is well-defined and follows the convention of providing a bilingual name for the directive it represents. TheGetNamesmethod correctly returns the staticNamesfield.src/ScriptEngine/Machine/Contexts/BslPredefinedAnnotation.cs (1)
- 14-19: The
NameMatchesmethod inBslPredefinedAnnotationcorrectly uses theHasNamemethod fromBilingualStringto perform a case-insensitive comparison of annotation names. This is a good use of existing functionality to ensure consistent behavior across the application.src/OneScript.Language/Localization/BilingualString.cs (1)
- 59-62: The
HasNamemethod inBilingualStringcorrectly checks for equality of the name in both Russian and English, using the providedStringComparisonparameter. This allows for flexible comparison strategies and is a good addition for supporting bilingual functionality.src/ScriptEngine/Machine/Contexts/PredefinedDirectiveHandler.cs (3)
24-34: The constructor of
PredefinedDirectiveHandlercorrectly initializes the_allLineContentLexerand_knownNamesfields. The use ofStringComparer.CurrentCultureIgnoreCasein theHashSetinitialization ensures case-insensitive comparison, which is consistent with the rest of the application's localization approach.36-38: The
DirectiveSupportedmethod properly checks if a directive is known by using the_knownNamesHashSet. This is an efficient way to check for the existence of an item in a collection.41-62: The
ParseAnnotationInternalmethod correctly parses annotations and adds an error to theErrorSinkif there is unexpected content after the directive. This is a good practice for ensuring the correctness of the parsed directives.src/ScriptEngine/Machine/Contexts/UserIterableContextInstance.cs (2)
34-53: The
CheckModuleCompatibilitymethod inUserIterableContextInstancecorrectly checks for the presence of theGetIteratormethod and throws aRuntimeExceptionif it is not found or does not meet the expected criteria. This ensures that the module adheres to the required interface for iterables.85-99: The
GetEnumeratormethod correctly creates aScriptedEnumeratorWrapperif the script method call returns aUserScriptContextInstance. This is a good implementation of theIEnumerableinterface, allowing for custom iteration logic in user scripts.src/ScriptEngine/Machine/Contexts/ScriptedEnumeratorWrapper.cs (4)
43-62: The
CheckCompatibilitymethod inScriptedEnumeratorWrappercorrectly checks for the presence and signatures of required methods. It throws a custom exception if the methods are missing or do not match the expected interface, which is crucial for the correct functioning of the enumerator.91-93: The
Resetmethod throwsNotSupportedException, which is standard practice for enumerators in .NET that do not support resetting. This is an appropriate implementation.96-102: The
Currentproperty getter correctly calls the script method and casts the result toBslValue. This ensures that the enumerator can provide the current item in the iteration.107-110: The
Disposemethod conditionally calls a script method if the_onDisposeMethodis found. This is a good practice for cleaning up resources in a managed way.src/ScriptEngine/Machine/Contexts/AttachedScriptsFactory.cs (1)
- 216-225: The
ScriptFactorymethod now conditionally creates aUserIterableContextInstanceif the module has the appropriate attributes. This is a good implementation of the factory pattern, allowing for the creation of different types of script context instances based on module attributes.
| services.RegisterEnumerable<IExecutorProvider, StackMachineExecutor>(); | ||
| services.RegisterEnumerable<IDirectiveHandler, ConditionalDirectiveHandler>(); | ||
| services.RegisterEnumerable<IDirectiveHandler, RegionDirectiveHandler>(); | ||
| services.RegisterEnumerable<IDirectiveHandler, RegionDirectiveHandler>(); |
There was a problem hiding this comment.
The addition of a duplicate registration for IDirectiveHandler, RegionDirectiveHandler in the SetDefaultOptions method seems like an oversight. Duplicate registrations can lead to unexpected behavior and should be avoided unless intentionally designed for a specific purpose.
- services.RegisterEnumerable<IDirectiveHandler, RegionDirectiveHandler>();Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| services.RegisterEnumerable<IExecutorProvider, StackMachineExecutor>(); | |
| services.RegisterEnumerable<IDirectiveHandler, ConditionalDirectiveHandler>(); | |
| services.RegisterEnumerable<IDirectiveHandler, RegionDirectiveHandler>(); | |
| services.RegisterEnumerable<IDirectiveHandler, RegionDirectiveHandler>(); | |
| services.RegisterEnumerable<IExecutorProvider, StackMachineExecutor>(); | |
| services.RegisterEnumerable<IDirectiveHandler, ConditionalDirectiveHandler>(); | |
| services.RegisterEnumerable<IDirectiveHandler, RegionDirectiveHandler>(); |
There was a problem hiding this comment.
Copy-pastes are evil. Thanks, Rabbit
There was a problem hiding this comment.
@EvilBeaver You're welcome! If there's anything else you need help with, feel free to ask. Happy coding! 🐇
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| public int Count() | ||
| { | ||
| if (_getCountMethod == null) | ||
| throw new RuntimeException( | ||
| new BilingualString( | ||
| "Класс не поддерживает получение количества элементов, т.к. в нем отсутствует метод "+GetCountName.Russian+"()", | ||
| "Class doesn't support items counting, because method "+GetCountName.English+"() is not defined") | ||
| ); | ||
|
|
||
| var ret = CallScriptMethod(_getCountMethod.DispatchId, Array.Empty<IValue>()); | ||
|
|
||
| return (int)ret.AsNumber(); |
There was a problem hiding this comment.
The Count method throws a RuntimeException if the _getCountMethod is not defined, which is appropriate for enforcing the contract of the ICollectionContext. However, the exception message should be consistent with the method being checked. The message references GetIteratorName instead of GetCountName.
- "Метод " + GetIteratorName.Russian + " должен быть функцией без параметров",
- "Method " + GetIteratorName.English + "() must be a function without parameters");
+ "Метод " + GetCountName.Russian + " должен быть функцией без параметров",
+ "Method " + GetCountName.English + "() must be a function without parameters");Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| public int Count() | |
| { | |
| if (_getCountMethod == null) | |
| throw new RuntimeException( | |
| new BilingualString( | |
| "Класс не поддерживает получение количества элементов, т.к. в нем отсутствует метод "+GetCountName.Russian+"()", | |
| "Class doesn't support items counting, because method "+GetCountName.English+"() is not defined") | |
| ); | |
| var ret = CallScriptMethod(_getCountMethod.DispatchId, Array.Empty<IValue>()); | |
| return (int)ret.AsNumber(); | |
| public int Count() | |
| { | |
| if (_getCountMethod == null) | |
| throw new RuntimeException( | |
| new BilingualString( | |
| "Метод " + GetCountName.Russian + " должен быть функцией без параметров", | |
| "Method " + GetCountName.English + "() must be a function without parameters") | |
| ); | |
| var ret = CallScriptMethod(_getCountMethod.DispatchId, Array.Empty<IValue>()); | |
| return (int)ret.AsNumber(); |
There was a problem hiding this comment.
@EvilBeaver, I understand that my previous comment may have come across as pedantic. My intention is to assist in maintaining clarity and consistency in the codebase. If you have any concerns or alternative suggestions, I'm here to help.
There was a problem hiding this comment.
No, go on. I'm just surprised how smart you are
There was a problem hiding this comment.
@EvilBeaver, thank you for the encouragement! If you need further assistance or have more changes to review, feel free to let me know. I'm here to help!
(\__/)
(•ㅅ•)
/ づ
| public static RuntimeException MissingMethod(BilingualString methodName, Exception parent = null) | ||
| { | ||
| var error = new BilingualString( | ||
| "Обязательный метод "+methodName.Russian+"отсутствует, или не соответствует интерфейсу итератора", | ||
| "Required method "+methodName.English+" is missing or doesn't match iterator interface"); | ||
|
|
||
| return new RuntimeException(error, parent); |
There was a problem hiding this comment.
The MissingMethod method constructs an error message that seems to have a typo in the Russian version. There should be a space between the method name and "отсутствует".
- "Обязательный метод "+methodName.Russian+"отсутствует, или не соответствует интерфейсу итератора",
+ "Обязательный метод "+methodName.Russian+" отсутствует, или не соответствует интерфейсу итератора",Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| public static RuntimeException MissingMethod(BilingualString methodName, Exception parent = null) | |
| { | |
| var error = new BilingualString( | |
| "Обязательный метод "+methodName.Russian+"отсутствует, или не соответствует интерфейсу итератора", | |
| "Required method "+methodName.English+" is missing or doesn't match iterator interface"); | |
| return new RuntimeException(error, parent); | |
| public static RuntimeException MissingMethod(BilingualString methodName, Exception parent = null) | |
| { | |
| var error = new BilingualString( | |
| "Обязательный метод "+methodName.Russian+" отсутствует, или не соответствует интерфейсу итератора", | |
| "Required method "+methodName.English+" is missing or doesn't match iterator interface"); | |
| return new RuntimeException(error, parent); |
|
Какой кайф.) Постараюсь потестить эту ситуацию в ближайщие дни.) |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (6)
- src/ScriptEngine/Hosting/EngineBuilderExtensions.cs (2 hunks)
- src/ScriptEngine/Machine/Contexts/AttachedScriptsFactory.cs (2 hunks)
- src/ScriptEngine/Machine/Contexts/IterableDirectiveProvider.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/PredefinedAnnotationHandler.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/ScriptedEnumeratorWrapper.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/UserIterableContextInstance.cs (1 hunks)
Additional comments: 2
src/ScriptEngine/Hosting/EngineBuilderExtensions.cs (1)
- 58-66: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [18-66]
The registration of
PredefinedAnnotationHandlerandIterableAnnotationProviderinSetDefaultOptionsis correct. Ensure that these registrations do not conflict with existing services and that all dependent components are aware of these new services.src/ScriptEngine/Machine/Contexts/AttachedScriptsFactory.cs (1)
- 213-229: The conditional instantiation of
UserIterableContextInstanceinScriptFactoryis a significant change. Verify that the logic correctly identifies modules with iterable attributes and that this change does not affect other parts of the system that rely onUserScriptContextInstance.
| public class IterableAnnotationProvider : IPredefinedAnnotationProvider | ||
| { | ||
| public static readonly BilingualString Names = new BilingualString("Обходимое", "Iterable"); | ||
|
|
||
| public BilingualString GetNames() | ||
| { | ||
| return Names; | ||
| } | ||
| } |
There was a problem hiding this comment.
The implementation of IterableAnnotationProvider is correct and follows the expected pattern for an annotation provider. Consider adding XML documentation comments to the class and its members to explain their purpose and usage within the system.
| public class PredefinedAnnotationHandler : ModuleAnnotationDirectiveHandler | ||
| { | ||
| private readonly ILexer _allLineContentLexer; | ||
| private readonly HashSet<string> _knownNames; | ||
|
|
||
| public PredefinedAnnotationHandler(IEnumerable<IPredefinedAnnotationProvider> providers, IErrorSink errorSink) : base(errorSink) | ||
| { | ||
| var builder = new LexerBuilder(); | ||
| builder.Detect((cs, i) => !char.IsWhiteSpace(cs)) | ||
| .HandleWith(new WordLexerState()); | ||
|
|
||
| _allLineContentLexer = builder.Build(); | ||
|
|
||
| _knownNames = new HashSet<string>(providers.SelectMany(provider => | ||
| new[] { provider.GetNames().Russian, provider.GetNames().English }), StringComparer.CurrentCultureIgnoreCase); | ||
| } | ||
|
|
||
| protected override bool DirectiveSupported(string directive) | ||
| { | ||
| return _knownNames.Contains(directive); | ||
| } | ||
|
|
||
| protected override void ParseAnnotationInternal(ref Lexem lastExtractedLexem, ILexer lexer, ParserContext parserContext) | ||
| { | ||
| var node = new AnnotationNode(NodeKind.Annotation, lastExtractedLexem); | ||
| _allLineContentLexer.Iterator = lexer.Iterator; | ||
|
|
||
| parserContext.AddChild(node); | ||
|
|
||
| // после ничего не должно находиться | ||
| var nextLexem = _allLineContentLexer.NextLexemOnSameLine(); | ||
| lastExtractedLexem = lexer.NextLexem(); // сдвиг основного лексера | ||
| if (nextLexem.Type != LexemType.EndOfText) | ||
| { | ||
| var err = LocalizedErrors.ExpressionSyntax(); | ||
| err.Position = new ErrorPositionInfo | ||
| { | ||
| LineNumber = node.Location.LineNumber, | ||
| ColumnNumber = node.Location.ColumnNumber, | ||
| Code = lexer.Iterator.GetCodeLine(node.Location.LineNumber), | ||
| ModuleName = lexer.Iterator.Source.Name | ||
| }; | ||
| ErrorSink.AddError(err); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public interface IPredefinedAnnotationProvider | ||
| { | ||
| BilingualString GetNames(); | ||
| } |
There was a problem hiding this comment.
The implementation of PredefinedAnnotationHandler and IPredefinedAnnotationProvider is correct. It would be beneficial to add XML documentation comments to explain the responsibilities of the handler, the interface, and their methods.
| public class UserIterableContextInstance : UserScriptContextInstance, ICollectionContext<BslValue> | ||
| { | ||
| private static BilingualString GetIteratorName = new BilingualString("ПолучитьИтератор", "GetIterator"); | ||
| private static BilingualString GetCountName = new BilingualString("Количество", "Count"); | ||
|
|
||
| private BslScriptMethodInfo _getCountMethod; | ||
| private BslScriptMethodInfo _getIteratorMethod; | ||
|
|
||
| public UserIterableContextInstance(IExecutableModule module, TypeDescriptor asObjectOfType, IValue[] args = null) : base(module, asObjectOfType, args) | ||
| { | ||
| CheckModuleCompatibility(module); | ||
| } | ||
|
|
||
| private void CheckModuleCompatibility(IExecutableModule module) | ||
| { | ||
| var error = new BilingualString( | ||
| "Модуль объекта Обходимое должен содержать функцию " + GetIteratorName.Russian + "()", | ||
| "Module of Iterable must contain function " + GetIteratorName.English + "()"); | ||
|
|
||
| try | ||
| { | ||
| _getIteratorMethod = (BslScriptMethodInfo)module.Methods.Single(method => | ||
| GetIteratorName.HasName(method.Name, StringComparison.CurrentCultureIgnoreCase)); | ||
| } | ||
| catch (InvalidOperationException e) | ||
| { | ||
| throw new RuntimeException(error, e); | ||
| } | ||
|
|
||
| if (!_getIteratorMethod.IsFunction() || _getIteratorMethod.GetBslParameters().Length != 0) | ||
| { | ||
| throw new RuntimeException(error); | ||
| } | ||
|
|
||
| _getCountMethod = (BslScriptMethodInfo)module.Methods.FirstOrDefault(method => | ||
| GetCountName.HasName(method.Name, StringComparison.CurrentCultureIgnoreCase)); | ||
|
|
||
| if (_getCountMethod != null) | ||
| { | ||
| var countError = new BilingualString( | ||
| "Метод " + GetCountName.Russian + " должен быть функцией без параметров", | ||
| "Method " + GetCountName.English + "() must be a function without parameters"); | ||
|
|
||
| if (!_getCountMethod.IsFunction() || _getCountMethod.GetBslParameters().Length != 0) | ||
| { | ||
| throw new RuntimeException(countError); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public int Count() | ||
| { | ||
| if (_getCountMethod == null) | ||
| throw new RuntimeException( | ||
| new BilingualString( | ||
| "Класс не поддерживает получение количества элементов, т.к. в нем отсутствует метод "+GetCountName.Russian+"()", | ||
| "Class doesn't support items counting, because method "+GetCountName.English+"() is not defined") | ||
| ); | ||
|
|
||
| var ret = CallScriptMethod(_getCountMethod.DispatchId, Array.Empty<IValue>()); | ||
|
|
||
| return (int)ret.AsNumber(); | ||
| } | ||
|
|
||
| public IEnumerator<BslValue> GetEnumerator() | ||
| { | ||
| var enumerator = CallScriptMethod(_getIteratorMethod.DispatchId, Array.Empty<IValue>()); | ||
| if (!(enumerator is UserScriptContextInstance userObject)) | ||
| { | ||
| throw ScriptedEnumeratorWrapper.IncompatibleInterfaceError(); | ||
| } | ||
|
|
||
| return new ScriptedEnumeratorWrapper(userObject); | ||
| } | ||
|
|
||
| IEnumerator IEnumerable.GetEnumerator() | ||
| { | ||
| return GetEnumerator(); | ||
| } |
There was a problem hiding this comment.
The implementation of UserIterableContextInstance is correct. It properly checks for the required methods and implements the ICollectionContext<BslValue> interface. Consider adding XML documentation comments to the class and its methods to explain their purpose and usage.
| DisposeMethodName = new BilingualString("ПриОсвобожденииОбъекта", "OnDispose"); | ||
|
|
||
| private BslScriptMethodInfo _moveNextMethod; | ||
| private BslScriptMethodInfo _getCurrentMethod; | ||
| private BslScriptMethodInfo _onDisposeMethod; | ||
|
|
||
| public ScriptedEnumeratorWrapper(UserScriptContextInstance userObject) | ||
| { | ||
| _userObject = userObject; | ||
| CheckCompatibility(); | ||
| } | ||
|
|
||
| private void CheckCompatibility() | ||
| { | ||
| _moveNextMethod = FindRequiredMethod(MoveNextMethodName); | ||
| _getCurrentMethod = FindRequiredMethod(GetCurrentMethodName); | ||
| _onDisposeMethod = FindOptionalMethod(DisposeMethodName); | ||
|
|
||
| if (!_moveNextMethod.IsFunction() || _moveNextMethod.GetBslParameters().Length != 0) | ||
| { | ||
| throw MissingMethod(MoveNextMethodName); | ||
| } | ||
|
|
||
| if (!_getCurrentMethod.IsFunction() || _getCurrentMethod.GetBslParameters().Length != 0) | ||
| { | ||
| throw MissingMethod(GetCurrentMethodName); | ||
| } | ||
|
|
||
| if (_onDisposeMethod != null && _onDisposeMethod.GetBslParameters().Length != 0) | ||
| { | ||
| throw MissingMethod(DisposeMethodName); | ||
| } | ||
| } | ||
|
|
||
| private BslScriptMethodInfo FindRequiredMethod(BilingualString names) | ||
| { | ||
| try | ||
| { | ||
| return (BslScriptMethodInfo)_userObject.Module.Methods.Single(m => | ||
| names.HasName(m.Name, StringComparison.CurrentCultureIgnoreCase)); | ||
| } | ||
| catch (InvalidOperationException e) | ||
| { | ||
| throw MissingMethod(names, e); | ||
| } | ||
| } | ||
|
|
||
| private BslScriptMethodInfo FindOptionalMethod(BilingualString names) | ||
| { | ||
| return (BslScriptMethodInfo)_userObject.Module.Methods.FirstOrDefault(m => | ||
| names.HasName(m.Name, StringComparison.CurrentCultureIgnoreCase)); | ||
|
|
||
| } | ||
|
|
||
| public bool MoveNext() | ||
| { | ||
| _userObject.CallAsFunction(_moveNextMethod.DispatchId, Array.Empty<IValue>(), out var result); | ||
| return result.AsBoolean(); | ||
| } | ||
|
|
||
| public void Reset() | ||
| { | ||
| throw new System.NotSupportedException(); | ||
| } | ||
|
|
||
| public BslValue Current | ||
| { | ||
| get | ||
| { | ||
| _userObject.CallAsFunction(_getCurrentMethod.DispatchId, Array.Empty<IValue>(), out var result); | ||
| return (BslValue)result; | ||
| } | ||
| } | ||
|
|
||
| object IEnumerator.Current => Current; | ||
|
|
||
| public void Dispose() | ||
| { | ||
| if (_onDisposeMethod != null) | ||
| _userObject.CallAsProcedure(_onDisposeMethod.DispatchId, Array.Empty<IValue>()); | ||
| } | ||
|
|
||
| public static RuntimeException IncompatibleInterfaceError() | ||
| { | ||
| var error = new BilingualString( | ||
| "Итератор не соответствует интерфейсу итератора", | ||
| "Iterator doesn't match Iterator interface"); | ||
|
|
||
| return new RuntimeException(error); | ||
| } | ||
|
|
||
| public static RuntimeException MissingMethod(BilingualString methodName, Exception parent = null) | ||
| { | ||
| var error = new BilingualString( | ||
| "Обязательный метод "+methodName.Russian+" отсутствует, или не соответствует интерфейсу итератора", | ||
| "Required method "+methodName.English+" is missing or doesn't match iterator interface"); | ||
|
|
||
| return new RuntimeException(error, parent); | ||
| } | ||
| } |
There was a problem hiding this comment.
The implementation of ScriptedEnumeratorWrapper is correct and adheres to the IEnumerator<BslValue> interface. Adding XML documentation comments would help clarify the purpose of each method and the class's role in the system.
Analysis Details0 Issues
Coverage and Duplications
|
@sfaqer это пока в ветке, с сайта не скачается, только если с билдсервера. Я скорее про решение вообще спрашивал - посмотри, как именно сделано |
|
@EvilBeaver а тестов на бсл нет или я в глаза долблюсь? |
Будут. Пока нет |
| private static readonly BilingualString | ||
| MoveNextMethodName = new BilingualString("ВыбратьСледующий", "MoveNext"); | ||
|
|
||
| private static readonly BilingualString | ||
| GetCurrentMethodName = new BilingualString("ТекущийЭлемент", "CurrentItem"); |
There was a problem hiding this comment.
Я вот тут не очень понял, выбрать следующий должен установить курсор на следующий элемент и его вернуть? Или просто курсор передвинуть? А если следующего элемента не оказалось что делать? У нас же нет заградительного "ЕстьСледующий()", ну и если ВыбратьСледующий возвращает элемент, то зачем нужен текущий элемент?
There was a problem hiding this comment.
Тут я так понимаю можно снять вопрос.
Если я правильно уловил логику то ВыбратьСледующий, проверяет есть ли следующий элемент и позиционирует курсор на следующий элемент, возвращая в качестве результат ответ на вопрос есть ли элемент. А ТекущийЭлемент возвращает то что указывает курсор.
Меня наверное нейминг немного смущает, из ВыбратьСледующий сложно сделать вывод что нужно булево вернуть, я в оригинальном ищью предлагал "ЕстьСледующий" и "Следующий", мне показалось оно так очевиднее, я не настаиваю, но чем не понравился нейминг из ишью?)
There was a problem hiding this comment.
Здесь интерфейс итератора C#, а не Явы. Мне он кажется в разы более удобным. Во-первых, мне не надо подстраиваться под него, имитируя апи явы снаружи, а во вторых, он и в целом адекватнее
There was a problem hiding this comment.
Имя Следующий вместо ВыбратьСледующий можно рассмотреть, да
|
|
||
| public IEnumerator<BslValue> GetEnumerator() | ||
| { | ||
| var enumerator = CallScriptMethod(_getIteratorMethod.DispatchId, Array.Empty<IValue>()); |
There was a problem hiding this comment.
_getIteratorMethod.DispatchId
Тут лежит не индекс метода в методах модуля, по которому потом он ищется.
Короче вот на это примере у функции ПолучитьИтератор() DispatchId = 1, и получает итератор он из функции Количество, потому что за первым индексом именно она лежит, а если второго метода нет то оно падает =)
There was a problem hiding this comment.
Это может быть, с индексами там всратая магия, мог напутать, спасибо
|
Я немного не понял почему различаются указания на коллекцию и иттератор, типа коллекция выделяет как директива компиляции а иттератор как аннотация над конструктором
Это так задумано? Нет возможности сделать единообразно?) |
Возможность есть, но разве мой вариант не удобнее раз в пять? Есть синтаксис аннотаций на модуль, его призываю юзать. И реализация со стороны шарпа проще |
Так а почему для итератора тогда другой?) |
@nixel2007 fyi надо бы учесть что аннотации модуля могут быть произвольные в подсветке и прочем |
Как понять "другой"? Для итератора вообще ничего не нужно, он просто должен иметь условленные методы. |
А, я походу уже сам додумал xD |
|
@sfaqer да, я уже осознал, что надо сделать демонстрирующие тесты как можно быстрее |
|
@sfaqer @nixel2007 а как вы смотрите вообще на отсутствие аннотации на модуле, а вместо этого такое:
|
Утиная имплементация, если что-то похоже на коллекцию значит это коллекция xD Мысль вообще интересная, только я пытаюсь понять не перебор ли это с магией, пока не могу понять. |
Зачем её тогда помечать аннотацией? Пускай у неё будет магическое имя. Старый код не сломается, так как никто не будет старые классы пихать в длякаждого. А для нового класса проще. |
|
P.S. Я пока не допираю, что вы имеете ввиду под "аннотациями на модуль". Такого концепта в языке не было раньше. |
#Использовать <- аннотация модуля |
Но это же инструкция... препроцессору... |
В шарповом коде это Module.Annotations так что нет xD |
Нет. Как и native это аннотация модуля. Но концепт существует только с 2.0, так что тут скорее моя вина, что я про это не кричал на каждом углу |
Но если спилить в пользу аннотации на конструкторе, тогда для модулей который не классы фича будет недоступна? |
А как ты себе представляешь "не-класс", который можно поместить в Для Каждого? |
Я не против ни одного из вариантов. В случае аннотаций модуля скорее всего надо надо будет доработать bsl-parser, но это не проблема. Не хочу, чтобы это было единственной причиной использовать аннотации над конструктором, если концепция аннотаций модуля здесь (и в последствии) удобнее. |
Я как раз и не уверен в том, что это удобнее. Попробовал набросать варианты и так и этак, не сказать, что аннотация в виде решетки однозначно и интуитивно понятна. В случае с нативом или с "версией языка" для условной "типизации" - понятно, а в случае с итераторами - нет. |
Я так понял мы в целом сейчас концепцию аннотаций модуля оскрипта обсуждаем, а не применительно к этой конкретной задачи, для текущей задачи разницы нет, да. А в целом при последующих добавлений аннотаций может вылезти |
|
А если вот такой синтаксис аннотации модуля: |
@sfaqer как раз нет. Здесь хотелось бы обсудить именно подход к конкретно этой задаче. Что будет более удобно |
Если мы ограничиваемся рамками юзер тайп коллешонс, то я думаю стандартные аннотации (через &) над конструкторами предпочтительнее как наиболее очевидная и понятная концепция |
Кажется это уже какой-то |
|
Сейчас еще вспомним про кастомные геттеры-сеттеры и кастомные индексаторы для доступа через |
А вот кстати для коллекций было бы не плохо, если они объявлены как индексированные конечно |
|
Потом ещё прикрутим переопределение мат операций (+ - / *) для классов и заживём
|
|
В общем, именно так. Как образец был
Также кажется необязательным отдельный класс для Итератора - функции |
Он и в текущем PR необязательный. Можно ведь вернуть ЭтотОбъект из ПолучитьИтератор() |
Оно как бы в первом приближении да, а во втором, когда у нас есть 10 тредов каждый из которых получает иттератор и ожидает что каждый из иттераторов обойдёт всю коллекцию, подобная реализация будет выглядеть дюже странно. |
Но его же можно добавить, даже пустой, просто чтобы повесить аннотацию, в той же ОСени эксплуатируется повсеместно |
EvilBeaver
changed the title
|
Парни, я адаптировал это в т.ч. под стиль ОСени, давайте согласуем имена волшебных методов и уже вольем |
Analysis Details5 Issues
Coverage and Duplications
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (30)
- src/OneScript.Core/Commons/Utils.cs (1 hunks)
- src/OneScript.Core/Contexts/BslMethodInfo.cs (2 hunks)
- src/OneScript.Core/Contexts/BslScriptMethodInfo.cs (2 hunks)
- src/OneScript.Core/Contexts/IPredefinedInterfaceChecker.cs (1 hunks)
- src/OneScript.Core/Contexts/PredefinedInterfaceRegistration.cs (1 hunks)
- src/OneScript.Core/Contexts/PredefinedInterfaceResolver.cs (1 hunks)
- src/OneScript.Core/Exceptions/InterfaceCheckException.cs (1 hunks)
- src/OneScript.Core/Execution/IExecutableModule.cs (2 hunks)
- src/OneScript.Core/Sources/EmptyModule.cs (2 hunks)
- src/OneScript.Language/Localization/BilingualString.cs (2 hunks)
- src/OneScript.Language/SyntaxAnalysis/SingleWordModuleAnnotationHandler.cs (1 hunks)
- src/OneScript.Native/Compiler/DynamicModule.cs (2 hunks)
- src/OneScript.Native/Compiler/NativeRuntimeAnnotationHandler.cs (1 hunks)
- src/ScriptEngine/Compiler/CompilerFrontend.cs (2 hunks)
- src/ScriptEngine/Hosting/EngineBuilderExtensions.cs (2 hunks)
- src/ScriptEngine/Machine/Contexts/AttachedScriptsFactory.cs (3 hunks)
- src/ScriptEngine/Machine/Contexts/ScriptedEnumeratorWrapper.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/UserIterableContextInstance.cs (1 hunks)
- src/ScriptEngine/Machine/Contexts/UserScriptContextInstance.cs (6 hunks)
- src/ScriptEngine/Machine/Interfaces/InterfaceRegistrationExtensions.cs (1 hunks)
- src/ScriptEngine/Machine/Interfaces/IterableBslInterface.cs (1 hunks)
- src/ScriptEngine/Machine/Interfaces/IterableBslInterfaceChecker.cs (1 hunks)
- src/ScriptEngine/Machine/Interfaces/IterablesModuleAnnotationsHandler.cs (1 hunks)
- src/ScriptEngine/Machine/Interfaces/IteratorBslInterface.cs (1 hunks)
- src/ScriptEngine/Machine/Interfaces/IteratorBslInterfaceChecker.cs (1 hunks)
- src/ScriptEngine/Machine/StackRuntimeModule.cs (1 hunks)
- src/Tests/OneScript.Core.Tests/PredefinedInterfacesTest.cs (1 hunks)
- tests/iterators/annotated_iterator.os (1 hunks)
- tests/iterators/collection_method_style.os (1 hunks)
- tests/iterators/test-iterators.os (1 hunks)
Files not reviewed due to errors (1)
- src/OneScript.Sources/EmptyModule.cs (no review received)
Files skipped from review due to trivial changes (1)
- src/OneScript.Core/Commons/Utils.cs
Additional comments: 39
tests/iterators/collection_method_style.os (1)
- 2-20: Ключевое слово
&Обходимоеиспользуется для аннотации класса как итерируемого. Это предполагает, что класс должен реализовать методы, необходимые для итерации, например,ПолучитьИтератор. В коде присутствует методПолучитьИтератор, который возвращает новый экземплярИтераторСАннотацией, что соответствует ожиданиям.tests/iterators/annotated_iterator.os (1)
- 2-24: Ключевое слово
&Итераториспользуется для аннотации класса как итератора. Это предполагает, что класс должен реализовать методы, необходимые для итерации, такие какВыбратьСледующийиТекущийЭлемент. В коде присутствуют оба этих метода, и они реализованы в соответствии с ожидаемым поведением итератора.src/OneScript.Core/Exceptions/InterfaceCheckException.cs (1)
- 13-22: Класс
InterfaceCheckExceptionпредставляет собой исключение, которое может быть выброшено при проверке соответствия интерфейсу. Конструкторы класса принимают сообщение об ошибке и, опционально, внутреннее исключение. Это стандартная практика для исключений в .NET, и здесь нет никаких проблем.src/ScriptEngine/Machine/Interfaces/IterableBslInterface.cs (1)
- 12-22: Класс
IterableBslInterfaceпредставляет интерфейс итерируемого объекта в BSL. Он содержит свойства для методовGetIteratorMethodиGetCountMethod, которые необходимы для реализации итерируемого объекта. Класс корректно определяет эти свойства как только для чтения, что является хорошей практикой для данных, которые не должны изменяться после инициализации.src/OneScript.Core/Contexts/IPredefinedInterfaceChecker.cs (1)
- 13-22: Интерфейс
IPredefinedInterfaceCheckerопределяет контракт для классов, которые будут проверять соответствие модулей предопределенным интерфейсам. МетодValidateпредназначен для выполнения этой проверки. Всё выглядит корректно и соответствует назначению интерфейса.src/OneScript.Core/Execution/IExecutableModule.cs (1)
- 29-29: Добавление свойства
Interfacesк интерфейсуIExecutableModuleрасширяет его функциональность, позволяя модулям иметь информацию о реализованных интерфейсах. Это изменение влияет на сигнатуру интерфейса и потенциально на логику и функциональность модулей, которые его реализуют. Необходимо убедиться, что все реализации интерфейса обновлены для поддержки этого нового свойства.Verification successful
Добавление свойства
Interfacesк интерфейсуIExecutableModuleбыло успешно проверено. Все реализации интерфейсаIExecutableModuleв файлахEmptyModule.cs,DynamicModule.csиStackRuntimeModule.csбыли обновлены для поддержки этого нового свойства. Следовательно, комментарий к коду подтверждается текущим состоянием кодовой базы.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Проверка наличия свойства Interfaces во всех реализациях IExecutableModule rg --type cs "class\s+\w+\s+:\s+IExecutableModule" --exec cat {} \; | rg "Interfaces"Length of output: 497
Script:
#!/bin/bash # Исправление скрипта для проверки наличия свойства Interfaces во всех реализациях IExecutableModule # Используем `xargs` для выполнения команды `cat` на файлах, найденных с помощью `rg` rg --type cs "class\s+\w+\s+:\s+IExecutableModule" -l | xargs rg "Interfaces"Length of output: 528
src/ScriptEngine/Machine/Interfaces/IteratorBslInterface.cs (1)
- 12-24: Класс
IteratorBslInterfaceкорректно определяет интерфейс итератора в BSL, включая методыMoveNextMethod,GetCurrentMethodиOnDisposeMethod. Эти свойства соответствуют ожидаемым методам итератора и правильно определены как только для чтения.src/ScriptEngine/Machine/Interfaces/IterablesModuleAnnotationsHandler.cs (1)
- 15-26: Класс
IterablesModuleAnnotationsHandlerрасширяетSingleWordModuleAnnotationHandlerи предоставляет обработку аннотаций модуля, связанных с итерируемыми объектами. ИспользованиеHashSetдля поддерживаемых имен аннотаций является эффективным способом обеспечения быстрого доступа к этим именам.src/OneScript.Native/Compiler/NativeRuntimeAnnotationHandler.cs (1)
- 18-29: Класс
NativeRuntimeAnnotationHandlerбыл изменен для наследования отSingleWordModuleAnnotationHandler. Это изменение упрощает логику обработки аннотаций, удаляя необходимость в методеParseAnnotationInternal. Теперь класс использует статическийHashSetдля определения поддерживаемых директив, что улучшает читаемость и поддержку кода.src/OneScript.Native/Compiler/DynamicModule.cs (1)
- 31-31: Добавление свойства
Interfacesк классуDynamicModuleпозволяет хранить и извлекать интерфейсы, связанные с модулем. Это изменение вводит новую функциональность в класс, что может потребовать дополнительной проверки в контексте всего приложения.Verification successful
Скрипт успешно нашел использование свойства
Interfacesв различных частях кода. Основываясь на полученных результатах, можно сделать вывод, что свойствоInterfacesиспользуется не только в классеDynamicModule, но и в других местах, включая тесты и другие классы, связанные с интерфейсами и их регистрацией. Это указывает на то, что добавление свойстваInterfacesвDynamicModuleможет влиять на эти части системы, и следует убедиться, что изменения совместимы с существующим использованием свойства.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Проверка использования свойства Interfaces в других частях кода rg --type cs "Interfaces" --exec cat {} \; | rg -v "DynamicModule"Length of output: 476
Script:
#!/bin/bash # Исправление скрипта для поиска использования свойства Interfaces в других частях кода rg --type cs "Interfaces" | grep -v "DynamicModule"Length of output: 9025
src/ScriptEngine/Machine/Interfaces/InterfaceRegistrationExtensions.cs (1)
- 17-37: Метод
EnablePredefinedIterablesдобавляет поддержку предопределенных итерируемых интерфейсов в DI контейнер. Это расширение позволяет легко включать поддержку итерируемых объектов в приложение. МетодыAddInterfaceиGetInterfaceпредоставляют удобный API для работы с интерфейсами в модулях.tests/iterators/test-iterators.os (1)
- 3-57: Сценарий тестирования
test-iterators.osвключает в себя несколько тестовых процедур, которые проверяют различные аспекты работы итераторов. Код тестов выглядит корректным и соответствует стандартам написания тестов для итераторов.src/OneScript.Core/Contexts/BslMethodInfo.cs (1)
- 25-25: Изменение уровня доступа поля
Annotationsсprivateнаprotectedпозволяет наследникам классаBslMethodInfoполучать доступ к аннотациям. Это изменение улучшает расширяемость и переиспользование кода.src/ScriptEngine/Machine/StackRuntimeModule.cs (1)
- 62-62: Добавление свойства
Interfacesк классуStackRuntimeModuleпозволяет хранить и извлекать интерфейсы, связанные с модулем. Это изменение вводит новую функциональность в класс, что может потребовать дополнительной проверки в контексте всего приложения.src/OneScript.Language/Localization/BilingualString.cs (1)
- 59-62: Метод
HasNameв классеBilingualStringпредоставляет функциональность для проверки соответствия имени в обоих языках, что улучшает поддержку локализации. Опциональный параметрcomparisonпозволяет настраивать тип сравнения, что делает метод гибким в использовании.src/ScriptEngine/Machine/Contexts/UserIterableContextInstance.cs (1)
- 22-65: Класс
UserIterableContextInstanceреализует интерфейсICollectionContext<BslValue>и предоставляет методыCountиGetEnumerator, необходимые для работы с итерируемыми объектами. Код соответствует ожидаемой логике итерируемых объектов и корректно обрабатывает возможные ошибки.src/ScriptEngine/Machine/Contexts/ScriptedEnumeratorWrapper.cs (1)
- 21-81: Класс
ScriptedEnumeratorWrapperреализует интерфейсIEnumerator<BslValue>и оборачивает пользовательский скриптовый объект для использования в качестве итератора. Класс корректно реализует методыMoveNext,Reset,CurrentиDispose, а также предоставляет статический метод для создания исключения несоответствия интерфейса. Всё выглядит корректно и соответствует ожиданиям.src/ScriptEngine/Machine/Interfaces/IterableBslInterfaceChecker.cs (1)
- 19-71: Класс
IterableBslInterfaceCheckerреализует интерфейсIPredefinedInterfaceCheckerи предоставляет логику для проверки соответствия модуля интерфейсу итерируемого объекта. Код корректно обрабатывает ошибки и добавляет интерфейс к модулю, если проверка проходит успешно.src/OneScript.Core/Contexts/PredefinedInterfaceRegistration.cs (1)
- 16-80: Класс
PredefinedInterfaceRegistrationпредоставляет механизм для регистрации предопределенных интерфейсов и аннотаций. Код корректно реализует методы сравнения и хэширования, что важно для использования в коллекциях.src/OneScript.Core/Contexts/PredefinedInterfaceResolver.cs (1)
- 18-82: Класс
PredefinedInterfaceResolverиспользуется для разрешения предопределенных интерфейсов в модулях. Код корректно обрабатывает регистрации интерфейсов и вызывает соответствующие проверки. Логика разрешения интерфейсов выглядит корректной и соответствует назначению класса.src/OneScript.Language/SyntaxAnalysis/SingleWordModuleAnnotationHandler.cs (1)
- 19-77: Класс
SingleWordModuleAnnotationHandlerобрабатывает аннотации модуля, состоящие из одного слова. Код корректно обрабатывает директивы и добавляет ошибки вErrorSink, если после директивы присутствует лишний текст. Это соответствует ожидаемому поведению обработчика аннотаций.src/ScriptEngine/Compiler/CompilerFrontend.cs (1)
- 5-43: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [27-79]
Изменения в классе
CompilerFrontendвключают добавление новых параметров в конструктор и использованиеPredefinedInterfaceResolverдля разрешения интерфейсов перед возвратом модуля. Это улучшает процесс компиляции, позволяя проверять соответствие модулей предопределенным интерфейсам.src/ScriptEngine/Hosting/EngineBuilderExtensions.cs (1)
- 64-64: Вызов
services.EnablePredefinedIterables()в методеSetDefaultOptionsдобавляет поддержку предопределенных итерируемых интерфейсов в DI контейнер. Это расширение позволяет легко включать поддержку итерируемых объектов в приложение.src/OneScript.Core/Contexts/BslScriptMethodInfo.cs (1)
- 122-127: Метод
HasBslAnnotationв классеBslScriptMethodInfoпредоставляет функциональность для проверки наличия аннотации BslAnnotationAttribute с использованием данногоBilingualString. Это улучшение позволяет более гибко работать с аннотациями, учитывая поддержку локализации.src/ScriptEngine/Machine/Interfaces/IteratorBslInterfaceChecker.cs (5)
- 21-30: Константы
IterableAnnotation,MoveNextMethodName,GetCurrentMethodName, иDisposeMethodNameиспользуют классBilingualStringдля поддержки локализации. Это соответствует целям PR по улучшению локализации.- 32-39: Метод
GetRegistrationsвозвращает регистрации интерфейсов, что соответствует ожидаемой функциональности для поддержки итераторов.- 41-44: Метод
ValidateвызываетCheckModule, что является стандартным подходом для проверки модуля на соответствие интерфейсу итератора.- 46-70: Метод
CheckModuleпроверяет, что методыMoveNext,CurrentItem, иOnDisposeсоответствуют требованиям интерфейса итератора. Важно, чтобы методы не принимали параметров, что соответствует спецификации итераторов.- 73-99: Методы
FindRequiredMethodиFindOptionalMethodиспользуют классBilingualStringдля поиска методов, что поддерживает локализацию. ИсключениеInterfaceCheckExceptionиспользуетBilingualStringдля сообщения об ошибке, что также соответствует целям PR.src/Tests/OneScript.Core.Tests/PredefinedInterfacesTest.cs (4)
- 21-49: Тест
Test_CheckerReactsOnModuleAnnotationsпроверяет реакцию проверяющего на аннотации модуля, что соответствует ожидаемому поведению для новой функциональности итераторов.- 53-76: Тест
Test_CheckerDoesNotReactsOnModuleAnnotationsпроверяет, что проверяющий не реагирует на отсутствие аннотаций, что также соответствует ожидаемому поведению.- 80-110: Тест
Test_CheckerReactsOnlyOnceOnManyAnnotationsубеждается, что проверяющий реагирует только один раз, даже если есть множество аннотаций, что предотвращает избыточную регистрацию.- 114-141: Тест
Test_CheckerReactsOnlyOnMethodAnnotationsпроверяет, что проверяющий реагирует только на аннотации методов, что соответствует ожидаемому поведению для работы с итераторами.src/ScriptEngine/Machine/Contexts/AttachedScriptsFactory.cs (1)
- 217-227: Изменения в методе
ScriptFactoryвключают условное создание экземпляраUserIterableContextInstanceв зависимости от наличия интерфейса итератора, что соответствует описанию изменений в AI-generated summary.src/ScriptEngine/Machine/Contexts/UserScriptContextInstance.cs (5)
- 26-35: Добавлены новые константы
BilingualStringдля терминов, связанных с итераторами, и удалены соответствующие строковые константы. Это улучшает поддержку локализации и соответствует целям PR.- 67-68: В методе
OnInstanceCreationиспользуются новые константыBilingualStringдля вызова скриптового метода, что улучшает читаемость и поддержку локализации.- 99-100: В методе
ActivateAsStringOverrideтакже используются новые константыBilingualString, что соответствует изменениям в остальной части файла.- 161-161: Использование константы
RaiseEventTermsв методеFindOwnMethodулучшает поддержку локализации и соответствует изменениям в остальной части файла.- 215-215: В методе
GetOwnMethodsDefinitionиспользуется новая константаRaiseEventTermsдля определения метода, что улучшает поддержку локализации и соответствует изменениям в остальной части файла.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- src/ScriptEngine/Machine/Interfaces/IteratorBslInterfaceChecker.cs (1 hunks)
- tests/iterators/annotated_iterator.os (1 hunks)
- tests/iterators/iterator.os (1 hunks)
- tests/iterators/test-iterators.os (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/ScriptEngine/Machine/Interfaces/IteratorBslInterfaceChecker.cs
- tests/iterators/annotated_iterator.os
- tests/iterators/test-iterators.os
Additional comments: 1
tests/iterators/iterator.os (1)
- 21-23: Функция
ТекущийЭлементвозвращает текущий элемент коллекции. Нет замечаний к реализации этой функции.
| Процедура ПриСозданииОбъекта(пКоллекция) | ||
| Коллекция = пКоллекция; | ||
| Индекс = 0; | ||
| Размер = Коллекция.Количество(); | ||
| КонецПроцедуры |
There was a problem hiding this comment.
Процедура ПриСозданииОбъекта корректно инициализирует переменные Коллекция, Индекс и Размер. Однако отсутствует проверка на то, что передаваемый параметр пКоллекция действительно поддерживает метод Количество. Это может привести к ошибке времени выполнения, если пКоллекция не имеет такого метода.
Процедура ПриСозданииОбъекта(пКоллекция)
+ Если ТипЗнч(пКоллекция) <> Тип("Массив") Тогда
+ ВызватьИсключение "Переданный параметр не является коллекцией.";
+ КонецЕсли;
Коллекция = пКоллекция;
Индекс = 0;
Размер = Коллекция.Количество();
КонецПроцедурыCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| Процедура ПриСозданииОбъекта(пКоллекция) | |
| Коллекция = пКоллекция; | |
| Индекс = 0; | |
| Размер = Коллекция.Количество(); | |
| КонецПроцедуры | |
| Процедура ПриСозданииОбъекта(пКоллекция) | |
| Если ТипЗнч(пКоллекция) <> Тип("Массив") Тогда | |
| ВызватьИсключение "Переданный параметр не является коллекцией."; | |
| КонецЕсли; | |
| Коллекция = пКоллекция; | |
| Индекс = 0; | |
| Размер = Коллекция.Количество(); | |
| КонецПроцедуры |
| Функция Следующий() | ||
| Если Индекс < Размер-1 Тогда | ||
| Индекс = Индекс + 1; | ||
| Возврат Истина; | ||
| Иначе | ||
| Возврат Ложь; | ||
| КонецЕсли; | ||
| КонецФункции |
There was a problem hiding this comment.
Функция Следующий предназначена для перехода к следующему элементу итератора. Однако условие Индекс < Размер-1 неверно, так как оно не позволит получить доступ к последнему элементу коллекции. Индекс последнего элемента равен Размер - 1, и итератор должен иметь возможность вернуть и его.
Функция Следующий()
- Если Индекс < Размер-1 Тогда
+ Если Индекс < Размер Тогда
Индекс = Индекс + 1;
Возврат Истина;
Иначе
Возврат Ложь;
КонецЕсли;
КонецФункцииCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| Функция Следующий() | |
| Если Индекс < Размер-1 Тогда | |
| Индекс = Индекс + 1; | |
| Возврат Истина; | |
| Иначе | |
| Возврат Ложь; | |
| КонецЕсли; | |
| КонецФункции | |
| Функция Следующий() | |
| Если Индекс < Размер Тогда | |
| Индекс = Индекс + 1; | |
| Возврат Истина; | |
| Иначе | |
| Возврат Ложь; | |
| КонецЕсли; | |
| КонецФункции |
There was a problem hiding this comment.
Но ведь индексация ведётся от нуля
There was a problem hiding this comment.
@nixel2007 Вы абсолютно правы, индексация начинается с нуля, и ваше замечание корректно. Условие Индекс < Размер-1 позволяет итератору корректно обработать все элементы коллекции, включая последний. Спасибо за уточнение.
Analysis Details4 Issues
Coverage and Duplications
|
@sfaqer приобщайся
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes