1
\$\begingroup\$

I have put together this small application that uses the JSONPlaceholder API and Vue.js.

The application crosses users and posts and displays the posts by user (author):

var app = new Vue({
  el: "#app",
  data: {
    base_url: "https://jsonplaceholder.typicode.com",
    uid: 1,
    currentUser: null,
    userData: null,
    users: [],
    posts: []
  },
  methods: {
    toggleActiveUser(user) {
      this.uid = user.id;
    },
    getUsers() {
      axios
        .get(`${this.base_url}/users`)
        .then((response) => {
          this.users = response.data;
        })
        .catch((error) => {
          console.log(error);
          this.errored = true;
        })
        .finally(() => (this.loading = false));
    },
    getCurrentUser() {
      axios
        .get(`${this.base_url}/users/${this.uid}`)
        .then((response) => {
          this.currentUser = response.data;
        })
        .catch((error) => {
          console.log(error);
          this.errored = true;
        })
        .finally(() => (this.loading = false));
    },
    getPosts() {
      axios
        .get(`${this.base_url}/posts?userId=${this.uid}`)
        .then((response) => {
          this.posts = response.data;
        })
        .catch((error) => {
          console.log(error);
          this.errored = true;
        })
        .finally(() => (this.loading = false));
    }
  },
  created() {
    this.getUsers();
    this.getCurrentUser();
    this.getPosts();
  },
  watch: {
    currentUser() {
      this.getCurrentUser();
    },
    posts() {
      this.getPosts();
    }
  },
  filters: {
    capitalize(value) {
      return value.charAt(0).toUpperCase() + value.slice(1);
    },
    titlecase(value) {
      return value.toLowerCase().replace(/(?:^|[\s-/])\w/g, function(match) {
        return match.toUpperCase();
      });
    },
    lowercase(value) {
      return value.toLowerCase();
    }
  }
});
.logo {
  width: 30px;
}

.nav-item {
  width: 100%;
}

@media (min-width: 768px) {
  .nav-item {
    width: auto;
  }
}

.post-grid {
  margin-top: 1rem;
  justify-content: center;
  display: flex;
  flex-wrap: wrap;
}

.post {
  display: flex;
  flex-direction: column;
  margin-bottom: 30px;
}

.post-container {
  flex: 1;
  padding: 15px;
  background: #fcfcfc;
  border: 1px solid #eaeaea;
}

.post-title {
  font-size: 18px !important;
}

.short-desc {
  text-align: justify;
}


/* User Info */

.table.user-info {
  margin-bottom: 0;
}

.table.user-info>tbody>tr:first-child>td {
  border-top: none !important;
}

.table.user-info td {
  padding: 6px !important;
}

.table.user-info tr td:first-child {
  font-weight: 500;
  width: 32% !important;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/axios/0.21.1/axios.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.6.11/vue.min.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.5.2/js/bootstrap.min.js"></script>
<link href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/4.4.1/css/bootstrap.min.css" rel="stylesheet" />

<div id="app">
  <nav class="navbar navbar-expand-md bg-dark navbar-dark">
    <!-- Brand -->
    <a class="navbar-brand p-0" href="#">
      <img src="https://www.pngrepo.com/png/303293/180/bootstrap-4-logo.png" alt="" class="logo">
    </a>
    <button class="navbar-toggler d-md-none ml-auto" type="button" data-toggle="collapse" data-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false" aria-label="Toggle navigation">
      <span class="navbar-toggler-icon"></span>
    </button>

    <!-- Links -->
    <div class="navbar-nav navbar-expand-md  w-100">
      <ul class="navbar-nav navbar-expand-md collapse navbar-collapse" id="navbarSupportedContent">
        <li class="nav-item">
          <a class="nav-link" href="#">Home</a>
        </li>
        <!-- Dropdown -->
        <li class="nav-item dropdown nav-item">
          <a class="nav-link dropdown-toggle" href="#" id="navbardrop" data-toggle="dropdown">Authors</a>
          <div class="dropdown-menu" v-if="users">
            <a class="dropdown-item" href="#" v-for="user in users" :key="user.id" @click="toggleActiveUser(user)">{{user.name}}</a>
          </div>
        </li>
      </ul>
    </div>
  </nav>

  <div class="container">

    <h1 class="h4 mt-3">Posts by {{currentUser?.name}}</h1>

    <div v-if="currentUser != null">
      <button type="button" class="btn btn-md btn-success" data-toggle="modal" data-target="#exampleModalCenter">
      More about this author
    </button>
    </div>

    <div class="row post-grid" v-if="posts">
      <div class="col-sm-6 post" v-for="post in posts">
        <div class="post-container">
          <h2 class="display-4 post-title">{{post.title | titlecase}}</h2>
          <div class="short-desc">
            {{post.body | capitalize}}
          </div>
        </div>
      </div>
    </div>
  </div>

  <div v-if="currentUser != null" class="modal fade" id="exampleModalCenter" tabindex="-1" role="dialog" aria-labelledby="exampleModalCenterTitle" aria-hidden="true">
    <div class="modal-dialog modal-dialog-centered" role="document">
      <div class="modal-content">
        <div class="modal-header">
          <h5 class="modal-title" id="exampleModalLongTitle">{{currentUser?.name}}</h5>
          <button type="button" class="close" data-dismiss="modal" aria-label="Close">
          <span aria-hidden="true">&times;</span>
        </button>
        </div>
        <div class="modal-body">
          <table class="table user-info">
            <tr>
              <td>Lives in</td>
              <td>{{currentUser?.address.city}}</td>
            </tr>
            <tr>
              <td>Address</td>
              <td>{{currentUser?.address.street}}, {{currentUser?.address.suite}}</td>
            </tr>
            <tr>
              <td>Zipcode</td>
              <td>{{currentUser?.address.zipcode}}</td>
            </tr>
            <tr>
              <td>Email</td>
              <td>
                <a href="mailto:{{currentUser?.email | lowercase}}">{{currentUser?.email | lowercase}}</a>
              </td>
            </tr>
          </table>
        </div>
        <div class="modal-footer">
          <button type="button" class="btn btn-secondary" data-dismiss="modal">Close</button>
        </div>
      </div>
    </div>
  </div>
</div>

Questions:

  1. Is the code verbose?
  2. Does the application leave a lot to be desired in regards to optimization?
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

1. Is the code verbose?

I would say it is a bit verbose. I first read over the methods to get data via XHR - e.g. users, posts, current user - and noted that they all have similar structure. I considered suggesting that there be one request to get all three types of data to reduce network traffic and allow simplifying the code, but I realize it is an external API being utilized, plus I noticed two of the three methods are called in watchers (see important point related to that in the next section), If that is necessary to have only certain data returned and you had control over the API endpoints then the endpoint could accept a list of data to return.

2. Does the application leave a lot to be desired in regards to optimization?

I would say yes - check this out: There is the method getCurrentUser which will set currentUser, and also there is an entry in watch for currentUser that calls this.getCurrentUser() - this typically leads to an infinite loop. The owners of the API likely would not like that! If it was my API and I was checking the logs it might look like a Denial of Service attack...

The same is true for posts. See this illustrated in the snippet below- notice the console logs that continue unceasingly (though they stop if the button labeled X Hide results is clicked).

Does the currentUser and posts change other than when the created lifecycle hook is called?

var app = new Vue({
  el: "#app",
  data: {
    base_url: "https://jsonplaceholder.typicode.com",
    uid: 1,
    currentUser: null,
    userData: null,
    users: [],
    posts: []
  },
  methods: {
    toggleActiveUser(user) {
      this.uid = user.id;
    },
    getUsers() {
      axios
        .get(`${this.base_url}/users`)
        .then((response) => {
          this.users = response.data;
        })
        .catch((error) => {
          console.log(error);
          this.errored = true;
        })
        .finally(() => (this.loading = false));
    },
    getCurrentUser() {
      axios
        .get(`${this.base_url}/users/${this.uid}`)
        .then((response) => {
          this.currentUser = response.data;
        })
        .catch((error) => {
          console.log(error);
          this.errored = true;
        })
        .finally(() => (this.loading = false));
    },
    getPosts() {
      axios
        .get(`${this.base_url}/posts?userId=${this.uid}`)
        .then((response) => {
          this.posts = response.data;
        })
        .catch((error) => {
          console.log(error);
          this.errored = true;
        })
        .finally(() => (this.loading = false));
    }
  },
  created() {
    this.getUsers();
    this.getCurrentUser();
    this.getPosts();
  },
  watch: {
    currentUser() {
      console.log('watch current user - calling getCurrentUser()');
      this.getCurrentUser();
    },
    posts() {
      console.log('watch current posts - calling getPosts()');
      this.getPosts();
    }
  },
  filters: {
    capitalize(value) {
      return value.charAt(0).toUpperCase() + value.slice(1);
    },
    titlecase(value) {
      return value.toLowerCase().replace(/(?:^|[\s-/])\w/g, function(match) {
        return match.toUpperCase();
      });
    },
    lowercase(value) {
      return value.toLowerCase();
    }
  }
});
.logo {
  width: 30px;
}

.nav-item {
  width: 100%;
}

@media (min-width: 768px) {
  .nav-item {
    width: auto;
  }
}

.post-grid {
  margin-top: 1rem;
  justify-content: center;
  display: flex;
  flex-wrap: wrap;
}

.post {
  display: flex;
  flex-direction: column;
  margin-bottom: 30px;
}

.post-container {
  flex: 1;
  padding: 15px;
  background: #fcfcfc;
  border: 1px solid #eaeaea;
}

.post-title {
  font-size: 18px !important;
}

.short-desc {
  text-align: justify;
}


/* User Info */

.table.user-info {
  margin-bottom: 0;
}

.table.user-info>tbody>tr:first-child>td {
  border-top: none !important;
}

.table.user-info td {
  padding: 6px !important;
}

.table.user-info tr td:first-child {
  font-weight: 500;
  width: 32% !important;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/axios/0.21.1/axios.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.6.11/vue.min.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.5.2/js/bootstrap.min.js"></script>
<link href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/4.4.1/css/bootstrap.min.css" rel="stylesheet" />

<div id="app">
  <nav class="navbar navbar-expand-md bg-dark navbar-dark">
    <!-- Brand -->
    <a class="navbar-brand p-0" href="#">
      <img src="https://www.pngrepo.com/png/303293/180/bootstrap-4-logo.png" alt="" class="logo">
    </a>
    <button class="navbar-toggler d-md-none ml-auto" type="button" data-toggle="collapse" data-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false" aria-label="Toggle navigation">
      <span class="navbar-toggler-icon"></span>
    </button>

    <!-- Links -->
    <div class="navbar-nav navbar-expand-md  w-100">
      <ul class="navbar-nav navbar-expand-md collapse navbar-collapse" id="navbarSupportedContent">
        <li class="nav-item">
          <a class="nav-link" href="#">Home</a>
        </li>
        <!-- Dropdown -->
        <li class="nav-item dropdown nav-item">
          <a class="nav-link dropdown-toggle" href="#" id="navbardrop" data-toggle="dropdown">Authors</a>
          <div class="dropdown-menu" v-if="users">
            <a class="dropdown-item" href="#" v-for="user in users" :key="user.id" @click="toggleActiveUser(user)">{{user.name}}</a>
          </div>
        </li>
      </ul>
    </div>
  </nav>

  <div class="container">

    <h1 class="h4 mt-3">Posts by {{currentUser?.name}}</h1>

    <div v-if="currentUser != null">
      <button type="button" class="btn btn-md btn-success" data-toggle="modal" data-target="#exampleModalCenter">
      More about this author
    </button>
    </div>

    <div class="row post-grid" v-if="posts">
      <div class="col-sm-6 post" v-for="post in posts">
        <div class="post-container">
          <h2 class="display-4 post-title">{{post.title | titlecase}}</h2>
          <div class="short-desc">
            {{post.body | capitalize}}
          </div>
        </div>
      </div>
    </div>
  </div>

  <div v-if="currentUser != null" class="modal fade" id="exampleModalCenter" tabindex="-1" role="dialog" aria-labelledby="exampleModalCenterTitle" aria-hidden="true">
    <div class="modal-dialog modal-dialog-centered" role="document">
      <div class="modal-content">
        <div class="modal-header">
          <h5 class="modal-title" id="exampleModalLongTitle">{{currentUser?.name}}</h5>
          <button type="button" class="close" data-dismiss="modal" aria-label="Close">
          <span aria-hidden="true">&times;</span>
        </button>
        </div>
        <div class="modal-body">
          <table class="table user-info">
            <tr>
              <td>Lives in</td>
              <td>{{currentUser?.address.city}}</td>
            </tr>
            <tr>
              <td>Address</td>
              <td>{{currentUser?.address.street}}, {{currentUser?.address.suite}}</td>
            </tr>
            <tr>
              <td>Zipcode</td>
              <td>{{currentUser?.address.zipcode}}</td>
            </tr>
            <tr>
              <td>Email</td>
              <td>
                <a href="mailto:{{currentUser?.email | lowercase}}">{{currentUser?.email | lowercase}}</a>
              </td>
            </tr>
          </table>
        </div>
        <div class="modal-footer">
          <button type="button" class="btn btn-secondary" data-dismiss="modal">Close</button>
        </div>
      </div>
    </div>
  </div>
</div>

Other comments

Style nit-pick - filter method names

I noticed there are filters like titlecase and lowercase which deviate from the camelCase style. I would prefer those be lowerCase and titleCase but that's just my opinion and you are free to ignore it.

Improve this answer
\$\endgroup\$
4
  • \$\begingroup\$ "This typically leads to an infinite loop". How would you fix it? \$\endgroup\$
    – Razvan Zamfir
    Jan 4, 2022 at 7:14
  • \$\begingroup\$ That depends on what would be required to “fix” it. When should it request new data? Should new data be requested when the value (e.g. of posts, currentUser) is something falsy like null/undefined Or even {}/[]? If so the watch callbacks can accept the new value as the first parameter (and old value as second parameter if needed) and use those accordingly \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ ♦
    Jan 5, 2022 at 7:37
  • \$\begingroup\$ What would your choice be? In other words, how would you (choose to) do it? \$\endgroup\$
    – Razvan Zamfir
    Jan 5, 2022 at 7:42
  • \$\begingroup\$ Unless someone can convince me that the watch callbacks are essential I’d remove those - and perhaps add code to handle the case where loading of the posts and current user doesn’t load an expected value - e.g. retry if acceptable up to a limit like 5 retries \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ ♦
    Jan 6, 2022 at 7:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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