Github user ruffle1986 commented on a diff in the pull request: https://github.com/apache/metron/pull/1172#discussion_r211960374 --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts --- @@ -15,63 +15,116 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import {Component, Input, Output, EventEmitter, OnInit, OnChanges, SimpleChanges} from '@angular/core'; +import {Component, Input, Output, EventEmitter, OnChanges, SimpleChanges} from '@angular/core'; +import { FormGroup, FormControl, Validators, ValidationErrors } from '@angular/forms'; + import * as moment from 'moment/moment'; import { DEFAULT_TIMESTAMP_FORMAT } from '../../utils/constants'; import { PcapRequest } from '../model/pcap.request'; +const DEFAULT_END_TIME = new Date(); --- End diff -- For the record, in this case, it would be ok to put these things inside the class definition because they're connected to the pcap filter so they would be hardly reused by other components, but who knows. It's hard to predict. I'm not sure about how these kind of things work in Java or other OOP heavy programming languages but Javascript (NodeJS) doesn't enforce us to put constants and static functions inside a class. The functions are static pure functions. They have nothing to do with the class' instance (this) so they should not be a private or a public class methods. they could be a static functions (a property of the class object itself). From testing perspective, I would keep it as is but export them of course so we wouldn't have to create a new instance of the component to test them. The best would be if they were replaced into a separate file in the appropriate folder based on whether it's shared across other components.
---