Naming conventions for methods, functions, and open api operations

[TangoYankee]

Description

Standardize the naming of methods, functions, and OpenAPI operations around some shared conventions.

1. REST-associated names start with a standardized verb associated with their http method.
   • Get -> find
   • Post -> create
   • Patch -> update
   • Put -> replace
   • Delete -> remove
2. Domain names are included based on whether it is used in its own domain (Domain as in tax lot or borough)
   • Within controllers, services, and repos domain names are excluded when the method is within that domain.
     • Domain names used outside of their home domain are stated explicitly
     • For example, within the zoning district domain the method to get a zoning district would be findById
     • Within the tax lot domain, the methods to get the zoning districts associated with a tax lot would be findZoningDistrictsByBbl
   • For operationIds, the domain is included because its variables are used at a global scope
     • The endpoint to get a zoning district by its id would be `findZoningDistrictById.
     • The endpoint to get zoning districts associated with a tax lot would be findZoningDistrictsByTaxLotBbl

• The domain rule applies to path parameters, as well.
  • The path parameter for tax lot bbls would be bbl when used within the tax lot domain but it would be taxLotBbl in the OpenAPI documenation

3. Lists of results are indicated either with plurality or the literal word 'Many'
   • Plurality is used when writing domain names outside of their home domain
     • ie) findZoningDistrictsByBbl when finding a list of zoning districts in the tax lot domain
   • "Many" is used when referring to the home domain (the domain name is omitted and we would like an alternate indicator)
     • 'Many' is preferred to 'All', because it's general enough to allow for filters/pagination and still make sense
     • ie) findMany when finding a list of boroughs in the borough domain
4. Classes are singular
   • Classes represent a template from which several instances may be created. They can be modeled more easily as singular.
   • File names are also singular, as they follow the name of the class
5. Variable names with corollary table columns should reflect the name of the column
   • zoningDistrictId v. zoningDistrictUuid has caused a bit of confusion. ZoningDistrictId should be used because the name of the column is id. uuid is merely the current format; it does not reflect the meaning of the variable.
   • taxLotBbl is fine because bbl is the column name for the primary id of tax lots.
   • To reinforce point 2, the variable names should include the domain name when used in a global context. This includes OpenApi documentation. The variable names should exclude the domain name when used in its home domain.
6. Non-default formats should be affixed to the "object" in the variable name
   • If getting the geojson of zoning districts by a tax lot bbl, it should be findZoningDistrictsGeoJsonByBbl
   • If getting the geojson format of a tax lot within the tax lot domain, the domain is excluded like so: findGeoJsonByBbl
   • If getting a list of geojson within the domains home, the object is "Many", like so: findManyGeoJson
   • The same rule would apply if we started supporting formats like csv
7. Initialisms that are subject to camelCase only capitalize the first letter.
   • ie) defaultUrl

Historical discussion

We should establish conventions around function names. The seed of this discussion is a comment on PR #114

Some feedback about method naming that I would love @TangoYankee's two cents on as well. I'm leaving this comment here as an example but this applies generally to what we name methods in services as well as repositories:

* I think we should have a convention of having some kind of verb in the name. So `zoningDistrictByTaxLotBbl` would be something like `findZoningDistrictByTaxLotBbl`.

* I think we should avoid including words in method and variable names that are redundant in the context of where the method/variable is defined. In the case of services and repositories, this usually means taking out words that are already in the class name of the class that the method belongs too. For example, if you look at where this method is being called, we do `this.taxLotRepo.zoningDistrictByTaxLotBbl(bbl);`. A good way to keep our method names from being too huge would be to simplify this to `zoningDistrictByBbl`, so you would do `this.taxLotRepo.zoningDistrictByBbl(bbl);`. This is still a good name because `taxLotRepo` tells you that you're dealing with taxLots.

* One other minor nit pick - this should probably be `zoningDistricts` not `zoningDistrict`, because it gives back a list of districts.

Putting these suggestions together, this method name might be something like findZoningDistrictsByBbl.

[TangoYankee]

I think we should avoid including words in method and variable names that are redundant in the context of where the method/variable is defined.

I think we should also consider the operationId in the OpenApi documentation. When writing the docs, I didn't appreciate that kubb would be generate schemas and type definitions from the operationId. It would be nice if the operationId and the controller methods matched each other as closely as possible. Those operation ids operate on a global scope. Consequently, it makes sense to include the domain in the operation name.

Examining an example controller and kubb zod + type, we can see how this may be structured. The code below currently exists on main. It has a zod schema of getTaxLotGeoJsonByBblPathParamsSchema. The reference operationId is getTaxLotGeoJsonByBbl. In contrast, the controller and service methods are findTaxLotByBblGeoJson. The operationId and controller method deviate in two key ways. First, they use different verbs, get v find. Second, they place geojson in different parts of the function name. We should refactor to be more consistent between the two. Though, we still want the controller to follow the don't include names that are implied by the domain context rule. A proposed refactor can be seen below.

code on main

  @Get("/:bbl/geojson")
  @UsePipes(new ZodValidationPipe(getTaxLotGeoJsonByBblPathParamsSchema))
  async findTaxLotByBblGeoJson(
    @Param() params: GetTaxLotGeoJsonByBblPathParams,
  ) {
    return this.taxLotService.findTaxLotByBblGeoJson(params.bbl);
  }

possible refactor

  @Get("/:bbl/geojson")
  @UsePipes(new ZodValidationPipe(findTaxLotByBblGeoJsonPathParamsSchema))
  async findByBblGeoJson(
    @Param() params: FindTaxLotByBblGeoJsonPathParams,
  ) {
    return this.taxLotService.findByBblGeoJson(params.bbl);
  }

There is also value in having an established mapping between the http method and the verb we want to use in the function names.

One possible map (with inspiration from nestjs sample)

• Get -> find
• Post -> create
• Patch -> update (deviates from nest example, follows more closely with the description of patch)
• Put -> replace (deviates from nest example, follows more closely with the description of puts)
• Delete -> remove

[TylerMatteo]

I agree that it makes sense to have the controller method names match the operation ID. I also agree that, if we're doing that, it makes sense to include the domains. After all, it's not like we actually invoke those methods in the code, so you never experience the clunkiness of taxLotController.getTaxLotByBbbl() anyway. So maybe we adopt the "don't include the domain in the method name" as a rule of thumb, but just for services and repositories?

The verb mapping you gave seems reasonable to me. I don't have a problem with preferring find over get. I suppose we could extend those verbs to services and repositories as well?

[TangoYankee]

Rereading my comment, I see the phrasing is ambiguous on whether to include the domains in the controller methods. I actually went with omitting them from the controller name. This is perhaps more apparent in the refactor example.

I figured that our brains will eventually need to start implicitly adding the 'domain' at some step of the chain from operationID to repository method. It might as well be from operationID to controller, rather than controller to service.

[TangoYankee]

I don't have a problem with preferring find over get

Yeah, all of the other methods map to some verb other than the method name. It "feels fair and consistent" to apply the same treatment to get.

I suppose we could extend those verbs to services and repositories as well?

Yes, please. I think the services already do and most of the repositories are yet to be created.

[TylerMatteo]

Oh, understood. I can see the logic there - may as well break the chain when you're going from a TS file to YAML, rather than breaking it when going from a TS file to....another TS file.

[TangoYankee]

One note on singular v plural. I agree with making the methods plural when they're lists/may contain several results. I like keeping the classes singular because, in my mind, they have similar conventions to database tables.