0
\$\begingroup\$

In a recruitment process, the company gives me a project to do in react native. I finished all the tasks. But company gave me a feedback and said that your project was good, but you did not apply best practices for using rest api. So we are not proceeding further with your application. I used redux with rest api. Below is the code of one screen. Please review it and suggest me how can I improve it

import React, {useState} from 'react';
import {View, StyleSheet, Alert} from 'react-native';
import {IconButton, TextInput, FAB} from 'react-native-paper';
import {useDispatch} from 'react-redux';
import Header from '../components/Header';

function AddCityScreen({navigation}) {
    const [cityName, setCityName] = useState('');
    const API_KEY = '799acd13e10b7a3b7cf9c0a8da6e5394';
    const dispatch = useDispatch();
    const citiesReducer = city => dispatch({type: 'ADD_CITY', payload: city});

    const onSaveCity = () => {
      getWeatherOfCity(cityName);
      navigation.goBack();
   };
   const getWeatherOfCity = async city => {
      try {
         const result = await fetch(
       `http://api.openweathermap.org/data/2.5/weather?q=${city}&units=metric&appid=${API_KEY}`,
        );

  if (result.status === 200) {
    const data = await result.json();
    citiesReducer(data);
  } else {
    Alert.alert('Error', 'Something went wrong while adding city', [
      {text: 'OK'},
    ]);
  }
} catch (ex) {
  Alert.alert('Error', 'Something went wrong while adding city', [
    {text: 'OK'},
  ]);
}
};
return (
  <>
  <Header navigation={navigation} titleText="Add a new city" />
  <IconButton
    icon="close"
    size={25}
    color="white"
    onPress={() => navigation.goBack()}
    style={styles.iconButton}
  />
  <View style={styles.container}>
    <TextInput
      label="Add City Here"
      value={cityName}
      mode="outlined"
      onChangeText={setCityName}
      style={styles.title}
    />
    <FAB
      style={styles.fab}
      small
      icon="check"
      disabled={cityName == '' ? true : false}
      onPress={() => onSaveCity()}
    />
  </View>
</>
);
}

const styles = StyleSheet.create({
  container: {
   flex: 1,
   backgroundColor: '#fff',
   paddingHorizontal: 20,
  paddingVertical: 20,
},
iconButton: {
  backgroundColor: 'rgba(46, 113, 102, 0.8)',
  position: 'absolute',
  right: 0,
  top: 40,
  margin: 10,
},
title: {
 fontSize: 24,
 marginBottom: 20,
},
text: {
  height: 300,
  fontSize: 16,
},
fab: {
 position: 'absolute',
 margin: 20,
 right: 0,
 top: 150,
 },
});

export default AddCityScreen;

      
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ Jun 25, 2021 at 0:06
  • \$\begingroup\$ I'm not sure what they could mean here with "applying rest api best practices". Maybe they mean not exposing API_KEY and storing this using something like react-native-keychain. Either way the variable could be defined outside the functional component scope. Apart from that I don't really see much wrong with what you've shown. Are you sure they found problems with this code specifically and not in some other piece of code? \$\endgroup\$ Jun 26, 2021 at 20:06

2 Answers 2

4
\$\begingroup\$

The first mistake: API key is public maybe you could store it in react-native-keychain.

Secondly, you should create three files for style(component), for view(component), for function(API&(component)). Then in the view component, you will import the style file and function file that have all algorithms.

\$\endgroup\$
1
\$\begingroup\$

The code looks good and clean but as per my evaluation following things should be taken care of

  • There should be folder structure for Component, Actions, Routers(APIs), Screens etc.
  • There should be common API class to handle responses like code 401(Unauthorized), 500(Bad Gateway), etc.

Hope this is helpful

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.