Conversation
rakeshM-Webonise
commented
Sep 3, 2019
- Added NetworkManager
- Added SQLITE Wrapper
| import { Platform, Alert } from "react-native"; | ||
| import { STRING_CONSTANTS } from "./string-constant"; | ||
|
|
||
| export default class Utility { |
There was a problem hiding this comment.
what is this class for?Lets not name any class or path as plain utility.js
There was a problem hiding this comment.
This class for common methods like date,time operations,alerts and null checks.
There was a problem hiding this comment.
lets not club them all together.The class will inflate and SRP(Single Responsibility Principle) will be broken
There was a problem hiding this comment.
for eg - break this class down into a another class like
export default class Alerter {
static showAlertWithMessage(title,message,navigation,onOkPressed,onCancelPressed){
Alert.alert(
title,
message,
[
{
text: STRING_CONSTANTS.OK_MSG,
onPress: () => onOkPressed()
}
],
{ cancelable: false }
);
}
}
There was a problem hiding this comment.
Utility should act as factory @ameyawebonise what you think ?
There was a problem hiding this comment.
then lets name such classes as xxxFactory.It should return a singleton instance of the class being created though
| import { MonoText } from "../components/StyledText"; | ||
| import NetworkManager from "../src/network/network-manager"; | ||
|
|
||
| export default class HomeScreen extends React.Component { |
There was a problem hiding this comment.
functional component
| @@ -1,32 +1,35 @@ | |||
| import React from 'react'; | |||
| import React from "react"; | |||
There was a problem hiding this comment.
file extension should be jsx for all components
| __DEV__ | ||
| ? require('../assets/images/robot-dev.png') | ||
| : require('../assets/images/robot-prod.png') | ||
| ? require("../assets/images/robot-dev.png") |
There was a problem hiding this comment.
extract this outside component as this will hit performance of component on update
| <TouchableOpacity onPress={this._handleHelpPress} style={styles.helpLink}> | ||
| <Text style={styles.helpLinkText}>Help, it didn’t automatically reload!</Text> | ||
| <TouchableOpacity | ||
| onPress={this._handleHelpPress} |
| @@ -88,101 +106,103 @@ export default class HomeScreen extends React.Component { | |||
| } | |||
|
|
|||
| _handleLearnMorePress = () => { | |||
There was a problem hiding this comment.
remove _ from all methods naming, it's not as per naming conventions
| } | ||
| } | ||
|
|
||
| //Reference - https://www.npmjs.com/package/expo-sqlite-orm/v/1.4.1 |
| Accept: 'application/json', | ||
| 'Content-Type': 'application/json', | ||
| }; | ||
|
No newline at end of file |
There was a problem hiding this comment.
no newline end of file