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

Coding Standard #6

Closed
haicon232 opened this issue Aug 28, 2017 · 6 comments
Closed

Coding Standard #6

haicon232 opened this issue Aug 28, 2017 · 6 comments
Labels

Comments

@haicon232
Copy link

  1. Sử dụng computed để tính toán 1 property khi không cần tham số tại các line sau:
    https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/components/partials/ImageBox.vue#L42
    https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/components/partials/ImageBox.vue#L43
    ...

  2. Sử dụng Array.prototype.map() trong những trường hợp for và không cần dùng đến index:
    Tham khảo:
    https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/map

VD:
https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/components/partials/Pagination.vue#L75

for(let i = 0; i < this.localPagRaw.length; i++) {
          let name = this.localPagRaw[i].match(/(?!")(\w+)(?=")/g)[0];
          let val = this.localPagRaw[i].match(/(?!page=)\d+/g)[0];
          this.localPag[name] = val;
}

Viết lại như sau:

this.localPagRaw.map((o) => {
      let name = o.match(/(?!")(\w+)(?=")/g)[0];
      let val = o.match(/(?!page=)\d+/g)[0];
      this.localPag[name] = val;
      return o
})

Ngoài ra chỗ này localPag em có thể sử dụng là result của cái map kia luôn. (Tùy thuộc vào logic)

  1. So sánh với undefined, null, ...
    VD:
    https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/components/partials/Pagination.vue#L22

Chỉ cần sử dụng !x là được.

  1. Chưa sử dụng JSLint trong Project.
  2. Viết code chưa thống nhất. Lúc thì default value là null, lúc thì default value là undefined.
  3. Sử dụng các prototype của Array.
    VD:
    https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/components/pages/Albumn.vue#L69

Nên sử dụng hàm
https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/push

  1. Tên biến nhiều chỗ chưa Camel Case.
    https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/components/IHeader.vue#L20

  2. Không commit key, password, ... lên git.
    Sử dụng biến config để load ra key, password ở đây
    https://github.com/hamzoni/Web-Front-End-Assignment/blob/master/src/assets/js/api.js#L4

@taquy
Copy link
Owner

taquy commented Aug 28, 2017

#1.
Before:

data() {
     return {
          thumbnail: this.info.urls.thumb,
          date: {
               created: DateFNS.format(DateFNS.parse(this.info.created_at), "hh:mm:ss A MMM Do YYYY"),
               updated: DateFNS.format(DateFNS.parse(this.info.updated_at), "hh:mm:ss A MMM Do YYYY")
          },
          fullName: this.info.user.first_name + " " + this.info.user.last_name,
          username: "@" + this.info.user.username,
          profileUrl: "/profile/" + this.info.user.username
     }
},

After:

computed: {
     thumbnail: function() {
          return this.info.urls.thumb;
     },
     date: function() {
          return {
               created: DateFNS.format(DateFNS.parse(this.info.created_at), "hh:mm:ss A MMM Do YYYY"),
               updated: DateFNS.format(DateFNS.parse(this.info.updated_at), "hh:mm:ss A MMM Do YYYY")
          }
     },
     fullName: function() {
          return this.info.user.first_name + " " + this.info.user.last_name;
     },
     username: function() {
          return "@" + this.info.user.username;
     },
     profileUrl: function() {
          return "/profile/" + this.info.user.username;
     }
}

#2.
Before:

for(let i = 0; i < this.localPagRaw.length; i++) {
    let name = this.localPagRaw[i].match(/(?!")(\w+)(?=")/g)[0];
    let val = this.localPagRaw[i].match(/(?!page=)\d+/g)[0];
    this.localPag[name] = val;
}

After:

this.localPagRaw.map(pg => {
    let name = pg.match(/(?!")(\w+)(?=")/g)[0];
    let val = pg.match(/(?!page=)\d+/g)[0];
    this.localPag[name] = val;
});

#3
Đúng rồi anh, check tồn tại short version. Đã fix x !== undefined thành x ạ.

#4 , #5 done

#6
Đã dùng Array.prototype.push() (alternation)

#7
Done.
Còn một số chỗ không có camel là của API hoặc tên component.

#8
cái này em ko rõ lắm, nhờ anh chỉ bảo thêm ạ.

taquy pushed a commit that referenced this issue Aug 28, 2017
@taquy taquy closed this as completed Aug 28, 2017
@hiendv
Copy link

hiendv commented Aug 28, 2017

  1. is not resolved

@hiendv hiendv reopened this Aug 28, 2017
@taquy taquy removed the enhancement label Aug 28, 2017
@taquy
Copy link
Owner

taquy commented Aug 29, 2017

I don't understand 4.

@hiendv
Copy link

hiendv commented Aug 29, 2017

Take a look at this: https://github.com/eslint/eslint

taquy pushed a commit that referenced this issue Aug 30, 2017
taquy pushed a commit that referenced this issue Aug 30, 2017
@hiendv
Copy link

hiendv commented Aug 30, 2017

Module build failed: Error: Cannot find module 'eslint-config-standard'

@hiendv
Copy link

hiendv commented Aug 30, 2017

/* eslint-disable no-new */
new Vue({
  el: '#app',
  render: h => h(App)
})

taquy pushed a commit that referenced this issue Aug 30, 2017
@hiendv hiendv closed this as completed Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants