[
https://issues.apache.org/jira/browse/AVRO-3001?focusedWorklogId=804117&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-804117
]
ASF GitHub Bot logged work on AVRO-3001:
----------------------------------------
Author: ASF GitHub Bot
Created on: 27/Aug/22 06:07
Start Date: 27/Aug/22 06:07
Worklog Time Spent: 10m
Work Description: rayokota commented on PR #1833:
URL: https://github.com/apache/avro/pull/1833#issuecomment-1229131964
> Hi @martin-g and @rayokota ,
Hi @KhrystynaPopadyuk ,
>
> Please find my 5 cents below.
>
> As was mentioned in description this is port from Java and it looks and
feels exactly as port. Of course this code will work. But it would be very
difficult maintain, extend and provide sufficient unit test coverage. This is
new functionality and it is easy refactor it now, and would be very difficult
update if later.
>
> Please find below few suggestion that, I believe, should be addressed:
>
> 1. Folder and file structure. For example:
>
> * Why do not create separate folder for JsonParser and placed all related
classes there
All the parsing classes are indeed in a separate `Parsing` package. The
`JsonEncoder` and `JsonDecoder` are siblings of `BinaryEncoder` and
`BinaryDecoder` in the `IO` package, which makes sense to me.
> * "ParsingDecoder"
(https://github.com/apache/avro/blob/c13a45debf4594dc7bc3c90aef64cee5ae115a5d/lang/csharp/src/apache/main/IO/ParsingDecoder.cs)
It states that this is "Base class for a -based" but Parser
(https://github.com/apache/avro/blob/c13a45debf4594dc7bc3c90aef64cee5ae115a5d/lang/csharp/src/apache/main/IO/Parsing/Parser.cs)
does no have base class. Moreover ParsingDecoder is base class for
JsonDecoder. So is "ParsingDecoder " correct name? What is the purpose of
having this class? Why we need this additional layer of abstraction?
The `ParsingEncoder` and `ParsingDecoder` are implementations of `Encoder`
and `Decoder` that maintain some parsing state. The `JsonEncoder` and
`JsonDecoder` subclass those classes to delegate the maintenance of the parsing
state. In the Java code, there are also `ValidatingEncoder` and
`ValidatingDecoder` that subclass `ParsingEncoder` and `ParsingDecoder`. I did
not port those classes over.
> * there are a lot of child classes and interfaces. For example file
Symbol.cs contains not only abstract class Symbol but 17 child classes and one
enum, And all definitions of fields, properties, child classes, enums, methods
are without any structure and order.
The `Symbol` class maintains the terminals and non-terminals of the parsing
grammar, so that is why there are a lot of classes and enums in it.
> * there are a lot of other things that need attention
>
> 2. Static method and classes is very controversial tool. It has own
benefits but also it can brink a lot of negative. Overall we should be use them
carefully and with attention. I have founded 38 static items in Symbol class.
That looks very suspicious.
I agree static methods and classes have their advantages and disadvantages,
but I believe there is nothing wrong in the use of them in the `Symbol` class.
I'd be happy to incorporate more specific suggestions at particular line
numbers.
> 3. Poor usage of interfaces but a lot of static and even hidden
dependencies. That should be reviewed in direction to use interfaces as
dependency instead of classes (even abstract). Dependency inversion/ dependency
injection can bring a lot of benefit and flexibility to code.
Dependency inversion also has its advantages and disadvantages, but I don't
think it would really help here, unless you can be more specific.
> 4. switch operator - is there way to reduce usage of switch/case?
I think `switch` can also have its advantages and disadvantages, but I think
its use is fine here.
Issue Time Tracking
-------------------
Worklog Id: (was: 804117)
Time Spent: 9.5h (was: 9h 20m)
> JsonEncode Decode support for C#
> --------------------------------
>
> Key: AVRO-3001
> URL: https://issues.apache.org/jira/browse/AVRO-3001
> Project: Apache Avro
> Issue Type: Improvement
> Components: csharp
> Affects Versions: 1.10.0, 1.11.0
> Reporter: Krishnan Unni
> Assignee: Robert Yokota
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.12.0
>
> Time Spent: 9.5h
> Remaining Estimate: 0h
>
> The C# library for avro currently supports only the Binary encoding and also
> with compile time types (Generic support only). As part of a project I am
> doing I need to validate the avro schema against the incoming json data on
> the fly without a predefined type (generated class). So basically comparing
> an avro schema (string/json representation) against a raw json string. It is
> possible with the Java library since it supports both non generic types and
> streams as well as json encoding. With C# currently this is not possible. Is
> there a plan to extend the C# library to provide these features? If yes, is
> there a timeline? If not is there any alternative to achieve this?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
