Update codebase to ensure idiomatic Java code

[franciszekjob]

Describe your changes

Update codebase to follow principles listed in readme i.e. ensure idiomatic Java code

• add @JvmOverloads to constructor in: FeltArray, Call
• add overloaded signDeployAccountV3, signDeclareV2, signDeclareV3 in Account
• add @JvmOverloads to constructors in data classes representing transactions
• remove TransactionFactory

Linked issues

Closes #403

Breaking changes

• This issue contains breaking changes

[DelevoXDG]

What about default constructors generated for data classes. I think they should have @JvmOverloads or secondary constructors as well, WDYT?

[franciszekjob]

Yup, I applied @JvmOverloads in Class and FeltArray constructors. Can't see any other data classes which need an overloaded constructor.

[DelevoXDG]

What I'm referring to are data class'es, e.g. InvokeTransactionV1, EmittedEvent or some TransactionPayload classes.

Which will have overloaded constructor with defaults in Kotlin, but won't have one in Java.

Come to think of it, I wonder if we shouldn't make some of these constructors private/internal:

• For instance, it's strange we're allowing to set TransactionType of something that is called InvokeTransactionV1
• Also, TransactionFactory should be used for creating Transaction objects anyways.
  • If we'd create overloaded constructors for these classes, we might consider removing TransactionFactory altogether for consistency

I understand that this might be too much. To satisfy the issue, I suppose it would be enough to just create necessary overloads / add @JvmOverloads. But we could also use this / another stacked PR to solve some related problems.

[franciszekjob]

I added @JvmOverloads to EmittedEvent, InvokeTransactionV1 and DeclareTransactionV3 .
I also created overloaded constructors in data classes representing transactions, and this way we could get rid of TransactionFactory.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 25.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 69.07%. Comparing base (d28b008) to head (8ce2900).
Report is 5 commits behind head on main.

❗ Current head 8ce2900 differs from pull request most recent head 6e719ac

Please upload reports for the commit 6e719ac to get more accurate results.

Files	Patch %	Lines
...n/kotlin/com/swmansion/starknet/account/Account.kt	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #463      +/-   ##
==========================================
- Coverage   71.12%   69.07%   -2.05%     
==========================================
  Files          75       75              
  Lines        3338     3247      -91     
  Branches      367      361       -6     
==========================================
- Hits         2374     2243     -131     
- Misses        798      849      +51     
+ Partials      166      155      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DelevoXDG]

What I'm referring to are data class'es, e.g. InvokeTransactionV1, EmittedEvent or some TransactionPayload classes.

Which will have overloaded constructor with defaults in Kotlin, but won't have one in Java.

Come to think of it, I wonder if we shouldn't make some of these constructors private/internal:

• For instance, it's strange we're allowing to set TransactionType of something that is called InvokeTransactionV1
• Also, TransactionFactory should be used for creating Transaction objects anyways.
  • If we'd create overloaded constructors for these classes, we might consider removing TransactionFactory altogether for consistency

I understand that this might be too much. To satisfy the issue, I suppose it would be enough to just create necessary overloads / add @JvmOverloads. But we could also use this / another stacked PR to solve some related problems.

[franciszekjob]

Instead of passing version, we can pass forFeeEstimate flag to the class constructor and based on that, indicate the version in the transaction.

1. It's more comfortable in usage.
2. It prevents creating transactions with wrong versions.

[franciszekjob]

We can get rid of setting default version, because now it's being set in private constructor based on forFeeEstimate param.