Azure / azure-sdk-for-java

This repository is for active development of the Azure SDK for Java. For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/java/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-java.
MIT License
2.32k stars 1.97k forks source link

Callautomation implementation design feedbacks #40903

Open anuchandy opened 3 months ago

anuchandy commented 3 months ago

There are two general design review feedback while reviewing azure-communication-callautomation changes (as part of azure-json migration) -

  1. CallAutomationEventParser: The parse method currently compares the eventType string with fully qualified event type name. The fully qualified name always starts with Microsoft.Communication.*. This means the logic may be changed to chrck if the eventType start with this value, if so, extract the last portion and then compare only the last portion. This saves us from repetitive string comparison for the values with same prefix.
  2. The converter types under converter namespace are nothing but custom model classes. The general pattern for such custom model is to keep them under implementation.models namespace and name appropriately.

Another observation I've while working on migrating to azure-json is – many of the public handwritten model types, that act as a custom model, uses serialization annotations while those are never used to create the payload to send over wire. Unless there are any specific reason, its recommended to keep them as plain models, this really ease the maintainability cost of library going forward, reduces the potential bugs and avoid unnecessary contract which we may not be able to implement consistently.

Another observation, the RecordKind in handwritten uses different casing than the one used by generated type RecordKind

anuchandy commented 3 months ago

Tagging owners to evaluate.