blackducksoftware / hub-detect

This is now deprecated. Please see synopsys-detect.
Apache License 2.0
38 stars 39 forks source link

creating an alternative to detect configuration being everywhere #318

Closed ekerwin closed 6 years ago

ekerwin commented 6 years ago

I was editing some code that involved some properties and thought this might be better to access property values.

The existing approach: detectConfiguration.getLongProperty(DetectProperty.DETECT_API_TIMEOUT)

What I'm suggesting: DetectProperty.DETECT_API_TIMEOUT.getLongValue()

In both approaches you need to know about the type of the property (Long, in this case) which seems fine.

The suggestion is a shorter, removes a dependency on detectConfiguration (which I think is desirable), but creates a headache for testing and is a little strange to set a static variable on an enum.

I don't think it is a slam duck improvement, but I'd like you guys to at least consider it to improve the legibility of the code involving properties and making it so the developer doesn't have to always focus on how tightly coupled DetectProperty and DetectConfiguration are.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 24.272% when pulling 47965c5fc0b0ec0e2d3108da5f18d9966f20458d on ek-property-enum into 978a7fd7265bbdfcb10ae243b97dbd7a4bd11cb7 on master.