[trello.com/c/qDelB45V] Add the height and the version to IPFS #657
Conversation
Adamant/Models/IPFSNodeStatus.swift
Outdated
| let timestamp = try container.decode(UInt64.self, forKey: .timestamp) | ||
| let timeStampInSeconds = String(timestamp / 1000) | ||
| let subString = timeStampInSeconds[ | ||
| timeStampInSeconds.index(timeStampInSeconds.startIndex, offsetBy: 2)..<timeStampInSeconds.endIndex |
There was a problem hiding this comment.
You can easily have crash here if startIndex + 2 >= endIndex. Please add guard check
There was a problem hiding this comment.
Not easily, closer to never, but yeah, just for safety it should be added
|
Please add PR description |
Is there any reasons to duplicate a descriptions if we pin task to a PR |
Adamant/Models/IPFSNodeStatus.swift
Outdated
| height = Int(string) ?? .zero | ||
| } else { | ||
| height = .zero | ||
| } |
There was a problem hiding this comment.
Don't use custom decoding. Use DTOs (even though custom decoding is used in legacy)
- DTO (Date Transfer Object) — the representation of the JSON without any app-related logics.
- Model — the model that is generated out of the DTO.
Check out NodesKeychainDTO
There was a problem hiding this comment.
In this simple case you don't even need a model, you can use DTO only
There was a problem hiding this comment.
I think there is no need to use DTO in our case because this model are already used to create a DTO from itself in func getStatusInfo(origin: NodeOrigin) async -> ApiServiceResult<NodeStatusInfo> and maps into a NodeStatusInfo. I've made this mapping in decoding to prevent these logics in the function above to prevent extra logic in it.
| ) | ||
| } | ||
| } | ||
|
|
||
| private func getHeightFrom(timestamp: UInt64) -> Int { | ||
| let timeStampInSeconds = String(timestamp / 1000) |
There was a problem hiding this comment.
Is not "timestamp" one word?
if one word - we need to make camel case in a right way.
If not - we need to make other properties to follow your case
VeretennikovSV
force-pushed
the
trello.com/c/qDelB45V
branch
from
40799b7 to
4cae3dc
Compare
Added mapping of timestamp to a height and version of IPFS