Feature/factory steps #3
Conversation
| { | ||
| string keyWord = SelectorUtils.GetKeyWordFromSelector(rootElement.Selector); | ||
| AutoElementData[] filteredChildren = rootElement.InnerChildrens | ||
| .Where(elm => !IsAppenderElement(elm)) |
There was a problem hiding this comment.
use just
.Where(IsAppenderElement)There was a problem hiding this comment.
but its the opposite of IsAppenderElement
| { | ||
| public interface IComponentFactoryCreatorStep : IComponentFactoryStep | ||
| { | ||
| IEnumerable<ComponentGeneratorOutput> InvokeStep(AutoElementData rootElement, IComponentFileCreator parent, IComponentFactoryCreatorStep next = null); |
There was a problem hiding this comment.
in some of the cases parent is null but based on the signature it's not it clear that it could be null.
either make all null-able parameters be null-able or add overloads with less parameters.
A better solution might be to add to override the method in ComponentFactoryCreatorStep and add virtual methods with less parameters
There was a problem hiding this comment.
i dont understand the changes needed to be done
There was a problem hiding this comment.
lets do that refactoring after I can better understand the tests and we have better code coverage...
| { | ||
| RunAppender(parentClassCreator, _classAppenderContainer.GetAppender(keyWord), current); | ||
| return GenerateClassesForElements(filteredChildren); | ||
| return true; | ||
| } | ||
|
|
||
| private IEnumerable<ComponentGeneratorOutput> GenerateClassesForElements(IEnumerable<AutoElementData> children, IComponentFileCreator parent = null) |
There was a problem hiding this comment.
this method is only called once without the second parameter. It does not need the parameter IComponentFileCreator parent = null
| { | ||
| RunAppender(parentClassCreator, _classAppenderContainer.GetAppender(keyWord), current); | ||
| return GenerateClassesForElements(filteredChildren); | ||
| return true; | ||
| } | ||
|
|
||
| private IEnumerable<ComponentGeneratorOutput> GenerateClassesForElements(IEnumerable<AutoElementData> children, IComponentFileCreator parent = null) | ||
| { | ||
| IEnumerable<ComponentGeneratorOutput> outputs = new List<ComponentGeneratorOutput>(); |
There was a problem hiding this comment.
use
new HashSet<ComponentGeneratorOutput>(new ComponentOutputComparer());instead. it's cleaner
| .ToArray(); | ||
|
|
||
| IComponentFileCreator current = _container.GetFileCreator(keyWord); | ||
| IEnumerable<ComponentGeneratorOutput> outputs = GenerateClassesForElements(children, current, next); |
There was a problem hiding this comment.
just use
new HashSet<ComponentGeneratorOutput>(new ComponentOutputComparer())| .Where(elm => !IsAppenderElement(elm)) | ||
| .ToArray(); | ||
| string keyWord = SelectorUtils.GetKeyWordFromSelector(selector); | ||
| return GenerateAppendersOutputs(keyWord, rootElement.InnerChildrens, next) |
There was a problem hiding this comment.
just use
var outputs = new HashSet<ComponentGeneratorOutput>(new ComponentOutputComparer());
outputs.UnionWith(GenerateAppendersOutputs(keyWord, rootElement.InnerChildrens, next));
outputs.UnionWith(GenerateFileCreatorOutputs(selector, filteredChildren, keyWord, next));
return outputs;| if (!IsAddinElement(child)) | ||
| { | ||
| IComponentFileCreator fileCreator = _container.GetFileCreator(child.Type); | ||
| if (fileCreator != null) |
There was a problem hiding this comment.
fileCreator is never null. if is doesn't find a creator it returns the default. (maybe create a NullFileCreator?)
There was a problem hiding this comment.
fileCreator could be null if you dont set anyone as default (when creating them) , maybe this case could happend in the tests - the builtinComponentsInserter makes sure there is a default
| .ForEach(att => RunAppender(parentClassCreator, att, current)); | ||
| var result = _creatorsSteps.FirstOrDefault(step => step.ShouldInvokeStep(current, parentClassCreator)) | ||
| ?.InvokeStep(current, parentClassCreator, this); | ||
| return result ?? new List<ComponentGeneratorOutput>(); |
There was a problem hiding this comment.
use
return result ?? Enumerable.Empty<ComponentGeneratorOutput>();
| @@ -58,7 +58,6 @@ public virtual ComponentGeneratorOutput GenerateComponentClass(string selector, | |||
| .AddCtor(CreateCtor(className)) | |||
| .SetClassName(className) | |||
| .SetNamesapce(NamespaceName) | |||
| .AddUsings(GetUsings(elements)) | |||
There was a problem hiding this comment.
it's one line 57, no need to add the usings twice
There was a problem hiding this comment.
didnt see that, you're right
i dont know if it was better , maybe the parameter :
was a mistake in the IComponentFactoryCreatorStep interface