Skip to content
This repository has been archived by the owner on May 28, 2019. It is now read-only.

Slight improvements. #39

Merged
merged 3 commits into from
Aug 29, 2018
Merged

Slight improvements. #39

merged 3 commits into from
Aug 29, 2018

Conversation

devsedo
Copy link
Contributor

@devsedo devsedo commented Aug 29, 2018

This PR provides slight improvement/shortening to a few places.

BitcoinPublicKey.swift

  • The idea is to use switch for data.first. Multiple comparings can be put in a single case.
  • data.first == nil equals data.isEmpty so both these cases can be put in default since we need to return false in case of being empty and in other unhandled cases as well.

Data+Hex.swift

  • The idea is to let initializers decide whether to return nil or not. Firstly we need to be able to parse UInt8 to ASCII letter. Then this ASCII letter can either be HEX digit or not.
  • Joining formatted strings requires less code.

Function.swift

  • String concatenation with comma-separator can be done more elegant.

@vikmeup
Copy link
Contributor

vikmeup commented Aug 29, 2018

@devsedo this is great! There is a few changes on this PR:
#29. @alejandro-isaza will take a look at this.

Thanks for contributing.

@devsedo
Copy link
Contributor Author

devsedo commented Aug 29, 2018

@vikmeup I was using my own git as a remote so I didn't notice that there was a commit to master before making this PR :/

@codecov-io
Copy link

codecov-io commented Aug 29, 2018

Codecov Report

Merging #39 into master will decrease coverage by 0.07%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
- Coverage   83.76%   83.69%   -0.08%     
==========================================
  Files          56       56              
  Lines        2045     2030      -15     
==========================================
- Hits         1713     1699      -14     
+ Misses        332      331       -1
Impacted Files Coverage Δ
Sources/Ethereum/Solidity/Function.swift 100% <100%> (ø) ⬆️
Sources/Data+Hex.swift 66.66% <100%> (-7.25%) ⬇️
Sources/Bitcoin/BitcoinPublicKey.swift 73.91% <80%> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c580c5...48af24c. Read the comment docs.

@alejandro-isaza alejandro-isaza merged commit b74cb7c into trustwallet:master Aug 29, 2018
@alejandro-isaza
Copy link
Contributor

Thanks!

@devsedo devsedo deleted the improvements branch August 29, 2018 15:16
@vikmeup
Copy link
Contributor

vikmeup commented Aug 29, 2018

@devsedo if you have some free time we always have some cool problem to work now. Feel free to ping me on telegram: @vikmeup

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants