-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/implement kyber #119
base: feature/implement-kyber
Are you sure you want to change the base?
Feature/implement kyber #119
Conversation
Built infrastructure to support token swap feature Token swap works (from eth to appc|eos|knc)
Cleaned Code
public SwapDataMapper swapDataMapper; | ||
@Inject SwapProofWriter swapBlockChainWriter; | ||
@Inject SwapProof swapProof; | ||
Spinner fromToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not injected variables can be private
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
// Another interface callback | ||
} | ||
|
||
public void getExpectedRate(View v) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use an OnClickListener instead of xml to listener click events
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
public void switchToMock1() { | ||
Intent myIntent = new Intent(this, SwapMock1Activity.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SwapMock1Activity should be the one creating its intent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@Override public BigInteger writeGetterSwapProof(SwapProof swapProof) { | ||
String from = swapProof.getFromAddress(); | ||
String to = swapProof.getToAddress(); | ||
Function getRates = new SwapDataMapper().getDataExpectedRate(swapProof); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SwapDataMapper should be injected on constructor of SwapBlockchainWriter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
import javax.inject.Inject; | ||
import org.web3j.utils.Convert; | ||
|
||
public class SwapMock1Activity extends AppCompatActivity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give a better name to this activity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
String data) { | ||
|
||
this.chainId = chainId; | ||
this.gasPrice = gasSettingsRepositoryType.getGasSettings(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't be here, the swapProff gas price value has to be sent over its constructor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -80,7 +82,9 @@ private void onError(ErrorEnvelope errorEnvelope) { | |||
@Override public void onClick(View v) { | |||
switch (v.getId()) { | |||
case R.id.save: { | |||
onSave(); | |||
//onSave(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SwapActivity should create its Intent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
android:layout_marginStart="56dp" | ||
android:layout_marginTop="32dp" | ||
android:onClick="testApprove" | ||
android:text="approve" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create a string on strings.xml file for this kind of text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
> | ||
|
||
<Button | ||
android:id="@+id/bt1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
give better names to this ids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -0,0 +1,105 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<android.support.constraint.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use constraints instead of margins to organize the view
Kyber use case implemented in a simple activity (SwapMock1Activity): get rates between ETHER and APPC and swap them.
Last version before changing to android mvp