tamotam-com / tamotam-app

(๐Ÿงช Early Beta) ๐Ÿค™ TamoTam. HangOut. Offline.
https://tamotam.com
35 stars 4 forks source link

Split up code to smaller pieces #43

Open danieldanielecki opened 1 year ago

danieldanielecki commented 1 year ago

Take a look into store, where actions and dispatchers are in a single file.

However, there's also feedback regarding EditEventScreen on https://github.com/AidOnline01/review-daniel/blob/main/1-huge-files.md. Similarly, other files could split up.

Example:

About the issue

There is a violation of Single Responsibility principle, which creates huge files and reduces maintability/testability/reusability of codebase.

Example

Example from the code [EditEventScreen](https://github.com/tamotam-com/tamotam-app/blob/master/screens/EditEventScreen.tsx) ```javascript import * as Location from "expo-location"; import analytics from "@react-native-firebase/analytics"; import crashlytics from "@react-native-firebase/crashlytics"; import useColorScheme from "../hooks/useColorScheme"; import Colors from "../constants/Colors"; import CustomMapStyles from "../constants/CustomMapStyles"; import DateTimePicker from "@react-native-community/datetimepicker"; import MaterialHeaderButton from "../components/MaterialHeaderButton"; import MapView, { Marker, PROVIDER_GOOGLE } from "react-native-maps"; import SelectImage from "../components/SelectImage"; import React, { useCallback, useEffect, useLayoutEffect, useRef, useState, Dispatch, MutableRefObject, SetStateAction, } from "react"; import StyledText from "../components/StyledText"; import { updateEvent } from "../store/actions/events"; import { useDispatch, useSelector } from "react-redux"; import { useNetInfo, NetInfoState } from "@react-native-community/netinfo"; import { ActivityIndicator, Alert, ColorSchemeName, Dimensions, KeyboardAvoidingView, Platform, ScrollView, StyleSheet, TextInput, } from "react-native"; import { Button } from "react-native-paper"; import { Coordinate } from "../interfaces/coordinate"; import { Event } from "../interfaces/event"; import { HeaderButtons, Item } from "react-navigation-header-buttons"; import { Region } from "../interfaces/region"; import { View } from "../components/Themed"; export default function EditEventScreen({ navigation, route }: any) { const colorScheme: ColorSchemeName = useColorScheme(); const dispatch: Dispatch = useDispatch>(); const eventId: number = route.params.eventId; const internetState: NetInfoState = useNetInfo(); const mapRef: MutableRefObject = useRef(null); const selectedEvent: Event = useSelector((state: any) => state.events.savedEvents.find((event: Event) => event.id === eventId) ); const [dateTimeMode, setDateTimeMode] = useState(""); const [descriptionValue, setDescriptionValue] = useState(""); const [error, setError] = useState(new Error()); const [initialRegionValue, setInitialRegionValue] = useState({ latitude: selectedEvent.latitude, longitude: selectedEvent.longitude, latitudeDelta: 10, longitudeDelta: 10, }); const [isLoading, setIsLoading] = useState(false); const [selectedDate, setSelectedDate] = useState( new Date(selectedEvent.date) instanceof Date ? new Date(selectedEvent.date) : new Date() ); const [selectedImage, setSelectedImage] = useState(""); const [selectedLocation, setSelectedLocation] = useState(); const [showDatepicker, setShowDatepicker] = useState(false); const [titleValue, setTitleValue] = useState(""); let markerCoordinates: Coordinate = { latitude: selectedEvent.latitude ? selectedEvent.latitude : 0, longitude: selectedEvent.longitude ? selectedEvent.longitude : 0, }; useEffect(() => { if (error.message !== "") { Alert.alert( "Unknown Error โŒ", "Please report this error by sending an email to us at feedback@tamotam.com. It will help us ๐Ÿ™\nError details: " + error.message + "\nDate: " + new Date(), [{ text: "Okay" }] ); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> useEffect[error], error: " + error, }); crashlytics().recordError(error); } }, [error]); useLayoutEffect(() => { navigation.setOptions({ headerLeft: () => ( navigation.goBack()} title="back" /> ), }); }, [navigation]); const getUserLocationHandler: () => Promise = useCallback(async () => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> getUserLocationHandler", }); setError(new Error("")); setIsLoading(true); if (Platform.OS !== "web") { const { status } = await Location.requestForegroundPermissionsAsync(); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> getUserLocationHandler, status: " + status, }); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> getUserLocationHandler, Platform.OS: " + Platform.OS, }); } try { const location: Location.LocationObject = await Location.getCurrentPositionAsync({}); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> getUserLocationHandler -> try, location: " + location, }); setInitialRegionValue({ latitude: location.coords.latitude, longitude: location.coords.longitude, latitudeDelta: 10, longitudeDelta: 10, }); } catch (error: unknown) { if (error instanceof Error) { Alert.alert( "Error โŒ", "We couldn't fetch your location.\nPlease give us the permissions, and it's essential to use TamoTam!", [{ text: "Okay" }] ); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> getUserLocationHandler -> catch, error: " + error, }); crashlytics().recordError(error); setError(new Error(error.message)); } } finally { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> getUserLocationHandler -> finally", }); setIsLoading(false); } }, [dispatch, setError, setIsLoading]); useEffect(() => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> useEffect[getUserLocationHandler]", }); getUserLocationHandler(); }, [getUserLocationHandler]); if (isLoading) { return ( ); } if (internetState.isConnected === false) { return ( Please turn on the Internet to use TamoTam. ); } if (selectedLocation) { markerCoordinates = { latitude: selectedLocation.latitude, longitude: selectedLocation.longitude, }; } const onDescriptionChange: (text: SetStateAction) => void = ( text: SetStateAction ) => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onDescriptionChange, text: " + text, }); setDescriptionValue(text); }; const onDateTimeChange: ( _event: any, selectedValueDate: Date | undefined ) => void = (_event: any, selectedValueDate: Date | undefined) => { if (_event.type === "dismissed") { setShowDatepicker(false); return; } analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onDateTimeChange, _event: " + _event, }); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onDateTimeChange, selectedValueDate: " + selectedValueDate, }); setSelectedDate(selectedValueDate); setShowDatepicker(false); }; const onImageChange: (imagePath: string) => void = (imagePath: string) => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onImageChange, imagePath: " + imagePath, }); setSelectedImage(imagePath); }; const onLocationChange: (e: { nativeEvent: { coordinate: Coordinate; }; }) => void = (e: { nativeEvent: { coordinate: Coordinate } }) => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onLocationChange, e.nativeEvent.coordinate: " + e.nativeEvent.coordinate, }); setSelectedLocation({ latitude: e.nativeEvent.coordinate.latitude, longitude: e.nativeEvent.coordinate.longitude, }); }; const onShowDatePicker: () => void = () => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onShowDatePicker, text: ", }); showDateTimeMode("date"); }; const onShowTimePicker: () => void = () => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onShowTimePicker, text: ", }); showDateTimeMode("time"); }; const onTitleChange: (text: SetStateAction) => void = ( text: SetStateAction ) => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onTitleChange, text: " + text, }); setTitleValue(text); }; const showDateTimeMode: (currentMode: string) => void = (currentMode: string) => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> showDateTimeMode, currentMode: " + currentMode, }); setShowDatepicker(true); setDateTimeMode(currentMode); }; const onSaveHandler: () => void = () => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onSaveHandler", }); setError(new Error("")); setIsLoading(true); try { const newEvent: Event = { id: eventId, date: selectedDate, description: descriptionValue ? descriptionValue : selectedEvent.description, imageUrl: selectedImage ? selectedImage : selectedEvent.imageUrl, isUserEvent: selectedEvent.isUserEvent, latitude: markerCoordinates.latitude, longitude: markerCoordinates.longitude, title: titleValue ? titleValue : selectedEvent.title, }; analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onSaveHandler -> try, newEvent: " + newEvent, }); dispatch(updateEvent(newEvent)); } catch (error: unknown) { if (error instanceof Error) { Alert.alert( "Error โŒ", "TamoTam couldn't save the changes.\nTry one more time!", [{ text: "Okay" }] ); analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onSaveHandler -> catch, error: " + error, }); crashlytics().recordError(error); setError(new Error(error.message)); } } finally { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> onSaveHandler -> finally", }); setIsLoading(false); } navigation.goBack(); }; const Map: () => JSX.Element = () => ( {markerCoordinates && ( )} ); return ( {!selectedEvent.latitude || !selectedEvent.longitude ? Problem with obtaining coordinates. : } Title Description {showDatepicker && ( )} Date: {new Date(selectedDate).toLocaleDateString()} Time: {new Date(selectedDate).toLocaleTimeString([], { hour: "2-digit", minute: "2-digit", })} ); } const styles = StyleSheet.create({ centered: { alignItems: "center", flex: 1, justifyContent: "center", }, dateTimeButtonsContainer: { flexDirection: "row", justifyContent: "space-around", marginVertical: 20, }, form: { marginHorizontal: 30, }, label: { fontSize: 18, marginBottom: 15, }, map: { height: Dimensions.get("window").height / 2, width: Dimensions.get("window").width, }, screen: { flex: 1, }, textInput: { borderBottomColor: "#ccc", borderBottomWidth: 1, marginBottom: 15, paddingVertical: 4, paddingHorizontal: 2, }, title: { fontSize: 20, fontWeight: "bold", textAlign: "center", }, }); ```
How it could have been done? ```javascript import analytics from "@react-native-firebase/analytics"; import React, { useCallback, useEffect, useLayoutEffect, useState, Dispatch, } from "react"; import { useDispatch, useSelector } from "react-redux"; import { useNetInfo, NetInfoState } from "@react-native-community/netinfo"; import { Event } from "../interfaces/event"; import { Region } from "../interfaces/region"; import LoadingView from '@/components/common/LoadingView.ts'; import InternetConnectionErrorView from '@/components/errors/InternetConnectionErrorView.ts'; import EditEventScreen from '@/components/edit-screen/EditEventScreen.ts'; import EditEventHeaderButtons from '@/components/edit-screen/EditEventHeaderButtons.ts'; import reportError from '@/services/reportError'; import userLocationHandlerCallback from '@/services/edit-event/userLocationHandlerCallback'; export default function EditEventScreen({ navigation, route }: any) { const dispatch: Dispatch = useDispatch>(); const eventId: number = route.params.eventId; const internetState: NetInfoState = useNetInfo(); const selectedEvent: Event = useSelector((state: any) => state.events.savedEvents.find((event: Event) => event.id === eventId) ); const [error, setError] = useState(new Error()); const [initialRegionValue, setInitialRegionValue] = useState({ latitude: selectedEvent.latitude, longitude: selectedEvent.longitude, latitudeDelta: 10, longitudeDelta: 10, }); const [isLoading, setIsLoading] = useState(false); useEffect(() => { reportError(error, 'screens -> EditEventScreen -> useEffect[error]') }, [error]); useLayoutEffect(() => { navigation.setOptions({ headerLeft: () => , }); }, [navigation]); const getUserLocationHandler: () => Promise = useCallback(() => userLocationHandlerCallback(setError, setIsLoading, setInitialRegionValue), [dispatch, setError, setIsLoading]); useEffect(() => { analytics().logEvent("custom_log", { description: "--- Analytics: screens -> EditEventScreen -> useEffect[getUserLocationHandler]", }); getUserLocationHandler(); }, [getUserLocationHandler]); if (isLoading) return if (internetState.isConnected === false) return return ( ); } ```

Changes

So instead of putting everything into one file, I would have splitted it into multiple smaller files (EditEventScreenStyles.ts, LoadingView.ts, InternetConnectionErrorView.ts, EditEventScreen.ts, reportError, EditEventHeaderButtons, userLocationHandlerCallback) and renamed it to the EditEventView.ts.

danieldanielecki commented 1 year ago

Maybe also https://www.reddit.com/r/reduxjs/comments/xnuz66/technical_feedback_for_react_native_mobile/ can help

danieldanielecki commented 1 year ago

which is basically React Toolkit from https://redux.js.org/introduction/why-rtk-is-redux-today

danieldanielecki commented 1 year ago

Actions have split in https://github.com/tamotam-com/tamotam-app/commit/55e8f933c611d7bae936b957044ce88048b6d791 which significantly improved performance. Now, each HTTP request is independent and speeds up the load of markers on the map.

danieldanielecki commented 1 year ago

Last piece of that improvement has been incorporated in https://github.com/tamotam-com/tamotam-app/commit/ad566add9d5c50afb68e64266bb81a81eda121ed

danieldanielecki commented 1 year ago

Now, only splitting component to smaller pieces is needed.