[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4605: Stats over http parse

[GitBox <gi...@apache.org>]

rob05c commented on a change in pull request #4605: Stats over http parse
URL: https://github.com/apache/trafficcontrol/pull/4605#discussion_r406312556
 
 

 ##########
 File path: lib/go-tc/crstates.go
 ##########
 @@ -25,8 +25,8 @@ import (
 
 // CRStates includes availability data for caches and delivery services, as gathered and aggregated by this Traffic Monitor. It is designed to be served at an API endpoint primarily for Traffic Routers (Content Router) to consume.
 type CRStates struct {
-	Caches          map[CacheName]IsAvailable                       `json:"caches"`
-	DeliveryService map[DeliveryServiceName]CRStatesDeliveryService `json:"deliveryServices"`
+	Caches          map[string]IsAvailable             `json:"caches"`
 
 Review comment:
   How strongly do you feel about all these `string` changes?
   
   So, there were a couple reasons I started using strong typing. 
   
   For one, it self-documents `map[string]string`. I got tired of `//map[CacheName]DeliveryServiceName` everywhere. Worse, if the comments aren't there, it's not always obvious. Even here, `Caches map[string]IsAvailable` could actually be a map[DeliveryServiceName]IsAvailable` for some reason. It's unlikely, but `tc.CacheName` makes it clear for sure.
   
   The other big reason, was that I kept getting them wrong. I'd pass a Cache Group name to something that needed a Cache name. I know that sounds ridiculous and unlikely, but in all the complexity of things like parsing DS stats, I found myself doing it, multiple times. And worse, the failure often isn't obvious. A stat just doesn't get populated, or a lookup thinks a DS doesn't exist, and things _mostly_ work.
   
   That might seem ridiculous, but I really was doing it, repeatedly. Maybe I'm just not as clever as some people, but this kind of thing really helped me at least, to avoid subtle and sometimes very-hard-to-catch errors at IMO a relatively small verbosity cost.
   
   Would you object strenuously if we left these in, and kept using the strong types, to make the code a little more self-documenting and help catch subtle runtime bugs like this?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services