Thomas Brooks #2
Conversation
| const formattedData = dataFormatter(props.data); | ||
| const filteredData = search(formattedData, props.searchTerm, props.onlyPublic); | ||
| const treeData = treeFormatter(filteredData, props.showIcons, props.withSystemName); |
There was a problem hiding this comment.
If I were to redo this exercise, I think I would make a Formatter class which would take the data when initialized, and then have functions like "formatAsTree". Then this code would live together where it belongs.
|
|
||
| import { Schema } from "./data"; | ||
|
|
||
| export const dataFormatter = (data: Schema[]) : any[] => { |
There was a problem hiding this comment.
I started out trying to write this with TS, but it became too complicated and eventually to move forward I just used any. I would not normally use any, because that defeats the purpose. I liked the small amount of type-checking I did have, so I left it instead of making it straight js.
| if (parentKey === 'schema') { | ||
| Icon = TableOutlined; | ||
| } | ||
|
|
||
| if (item.systemName === 'forms') { | ||
| Icon = FormOutlined; | ||
| } | ||
|
|
||
| if (item.systemName === 'views') { | ||
| Icon = FilterOutlined; | ||
| } | ||
|
|
||
| if (item.systemName === 'columns') { | ||
| Icon = NumberOutlined; | ||
| } |
There was a problem hiding this comment.
Better: Make this a switch
Best: Make an IconFactory
| hasChildren = (currentNode.hasOwnProperty("children") && currentNode.children.length); | ||
|
|
||
| let matched = ( | ||
| searchTerm === '' || |
There was a problem hiding this comment.
One of the things I wanted to try to do but ran out of time for was to pass filters down to the search. I would create filters that have a call function: SearchFilter, PublicFilter, something like that. I was hoping to do something like filters.forEach(filter => filter.call()) There is some connectedness to the searchTerm and onlyPublic so this may not have worked as I wanted. But I wanted to make it more extensible, so new filters could be added.
| }; | ||
|
|
||
| const search = (data, searchTerm = '', onlyPublic = false) => { | ||
| var cloned = JSON.parse(JSON.stringify(data)); |
There was a problem hiding this comment.
To avoid changing our data, clone it. I think if this is not JSON I would have to do something more involved.
| let hasChildren; | ||
|
|
||
| currentNode.hasOwnProperty("children") && currentNode.children.forEach(child => { | ||
| let keepChild = filter(child, searchTerm, onlyPublic); |
There was a problem hiding this comment.
This is doing depth-first search recursively; in a refactor I would change it to iterative to avoid stack overflow.
No description provided.