Skip to content
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

Some ideas for refactoring duplication in admin pages #25

Open
tomduggan85 opened this issue Jul 30, 2018 · 0 comments
Open

Some ideas for refactoring duplication in admin pages #25

tomduggan85 opened this issue Jul 30, 2018 · 0 comments

Comments

@tomduggan85
Copy link

tomduggan85 commented Jul 30, 2018

Hi @razan99 @Balsam-Faysal @InassTubail @amusameh @NoureldeanSaed
There's a little bit of common / repeated jsx in the three admin pages (AddFlight, UpdateFlight, Flight) that looks like this:

<div className="container">
    <Header />
    <div className="sub-container-sidenav-form">
        <SideNav />
        <div className="sub-container-form">
        ...

Ideally, that common code could be moved / restructured so it doesn't have to be repeated on each page (the DRY principle - Don't Repeat Yourself). That's best because then if you need to change it, you only have to change it in one place (otherwise you'll change it everywhere and you might forget one of the places). But this is a small project and it won't have more than these 3 pages, so there are probably more important things to worry about :)

But just for fun, here are some ideas on how you could remove that duplication in the future / on other projects:

  1. Put that common code in a parent class and use inheritance to set up AddFlight, UpdateFlight, Flight, as child classes of it. The child classes then override part of the rendering with their specific render logic.
    The parent class looks like this (sorry for bad formatting)
class AdminPage extends React.Component {
  render() {
    return (
      <div className="container">
        <Header />
          <div className="sub-container-sidenav-form">
          <SideNav />
            <div className="sub-container-form">
              {this.renderPageContent()}
            <div>
       </div>
     </div>
    )
  }
  renderPageContent() {
    return null// Do nothing here, because the child classes will override this
  }
}

.. and the child looks something like this:

class UpdateFlight extends AdminPage {
  renderPageContent() {
    // All of the interesting stuff goes here 
    return (<DetailsCard .......
  }
  //Notice how UpdateFlight does not define a render() method, it relies on the AdminPage render method
}

OR

  1. Use a Higher-Order Component - a HOC (https://reactjs.org/docs/higher-order-components.html).
    Create a HOC function to wrap all of the pages in their common jsx structure, and then export the pages using that HOC. This is how react-redux's connect() function works.
    The HOC could look like this:
const withAdminComponent = ( PassedComponent ) => {
  render() {
    return (
      <div className="container">
        <Header />
          <div className="sub-container-sidenav-form">
          <SideNav />
            <div className="sub-container-form">
              <PassedComponent {...this.props} />
            <div>
       </div>
     </div>
    )
  }
}

And then UpdateFlight, Flight and AddFlight wouldn't need that jsx in their render functions, but would export themselves already wrapped in the HOC:

class UpdateFlight extends React.Component {
  ...
}
export default withAdminComponent(UpdateFlight)

OR
3) Nested router
You could have an AdminPage class that renders the same common jsx as shown above and then, in its render function, has a react-router <Switch> tag to conditionally display either UpdateFlight, Flight, or AddFlight. This is my least favorite option because there would still have to be routes in App.js, to handle other routes, and I think it's harder to understand an app when there are multiple react-router switches happening in the codebase.

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

No branches or pull requests

1 participant