1👍
When considering code quality, there are several criteria at play and, depending on the intended purpose of the code, their importance might vary:
- predictability of outcome
- reliability (probability that the software will run without failure)
- maintainability (how easily software can be maintained: size, consistency, structure and complexity)
- readability (how easy it is for other developers to understand what code does + consistence of style & linting rules) – this is typically measured by how confident a developer is to modify the existing code while accurately predicting the outcome
- portability (how usable the software is in different environments)
- reusabilty (how easy it is to reuse this code in different scenarios/environments/products)
- scalability (how easy it is to reuse this software in a larger/more complex system – often tied in with modularity)
- modularity (ability of code to include/exclude functional bits or to be included/excluded from a larger system)
- testability (how well the software supports testing efforts, including automation)
- browser compatibility
As you can see, a lot of the metrics overlap each other and most converge in the same point.
While in modern teams modularity and testability are typically held as higher order values, a programmer’s code quality is rarely actually tested, and it only happens when they’re considered for a position in a company. Funnily enough, in such reviews the code readability is usually the highest virtue, as it makes code reviews easier.
As far as your particular snippet goes, the only real question here is whether or not to have all buttons as children of the same parent or have them separated into two parent rows. Considering their position to each other (you want them in same position on all responsiveness intervals) the desired outcome should be to have them split between two parents.
Another argument for this choice is that in order to display them as required while they’re children of the same parent one would have to move the display logic from JavaScript into CSS and, because you have a larger pool of developers who are decent in JavaScript than those who are decent in CSS, having the logic in JavaScript should be preferred.
All the above considered, here’s an example of how to write the above snippet for passing the strictest code reviews in high profile coding companies:
Vue.config.devtools = false;
Vue.config.productionTip = false;
new Vue({
el: '#app',
data: () => ({
notes: ['A', 'B', 'C', 'D', 'E', 'F', 'G']
}),
computed: {
noteRows() {
return [
[2, 3, 4],
[5, 6, 0, 1]
].map(row => row.map(i => this.notes[i]))
}
},
methods: {
checkNote(note) {
console.log(note);
}
}
})
.row {
display: flex;
justify-content: space-between;
}
/* button styling, not really part of the solution */
.row button {
background-color: white;
border: 1px solid #CCC;
padding: .75em;
margin: .75em;
min-width: 16.67%;
border-radius: .25em;
cursor: pointer
}
.row button:hover {
border-color: #888;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<div id="app">
<div class="row" v-for="(row, key) in noteRows" :key="key">
<button v-for="note in row"
:key="note"
v-text="note"
@click="checkNote(note)" />
</div>
</div>
The obvious advantages are:
- high readability and predictability of outcome, even with junior developers
- avoids unnecessary use of localized conventions or variables
- reusable (you can basically move
noteRows
in either a helper file or a mixin and import it in multiple components) - doesn’t use expressions or features that would limit its browser compatibility
- simple and easy to style markup, even for coders with a limited understanding of CSS
If it’s unclear how to reuse the above code across multiple components, here’s how:
Vue.config.devtools = false;
Vue.config.productionTip = false;
Vue.use({
install(Vue) {
Vue.prototype.$notes = ['A', 'B', 'C', 'D', 'E', 'F', 'G'];
}
});
const noteRows = {
computed: {
noteRows() {
return [
[2, 3, 4],
[5, 6, 0, 1]
].map(row => row.map(i => this.$notes[i]))
}
}
};
// now $notes exists in any component
// and noteRows needs to be (imported and) specified as mixin
new Vue({
el: '#app',
mixins: [noteRows],
methods: {
checkNote(note) {
console.log(note);
}
}
})
.row {
display: flex;
justify-content: space-between;
}
/* button styling, not really part of the solution */
.row button {
background-color: white;
border: 1px solid #CCC;
padding: .75em;
margin: .75em;
min-width: 16.67%;
border-radius: .25em;
cursor: pointer
}
.row button:hover {
border-color: #888;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<div id="app">
<div class="row" v-for="(row, key) in noteRows" :key="key">
<button v-for="note in row" :key="note" v-text="note" @click="checkNote(note)" />
</div>
</div>
If you don’t need $notes
on prototype for anything other than noteRows
, I’d move it in the mixin. Not as data()
but as a computed
(it doesn’t need to be reactive) so there’s a minor performance improvement there. Emphasis on minor.
1👍
if you wanted to refactor, I think you could write your row into a component like so
<template>
<div>
<button class="note-btn note-btn-default" v-for="note in notes" >{{note}}</button>
</div>
</template>
<script>
export default{
props:['notes']
}
</script>
then use that component like so in your html.
<note-btn :notes="notes"></note-btn>
1👍
Try to use CSS grid as display for your row then use grid-row-start
to put the item at its row using class binding based on the note index :class="
${index<3?’first-row’:’second-row’}"
let app = new Vue({
el: '#app',
data: {
notes: ['A', 'B', 'C', 'D', 'E', 'F', 'G']
}
})
.row {
display: grid;
grid-template-columns: 1fr 1fr 1fr 1fr;
grid-template-rows: auto auto;
}
.first-row {
grid-row-start: 1;
}
.second-row {
grid-row-start: 2;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.17/vue.js"></script>
<div id="app">
<div class="row">
<button v-for="(note,index) in notes" :class="`${index<3?'first-row':'second-row'}`" :key="note">{{note}}
</button>
</div>
</div>
1👍
via CSS & flex
, you can use justify-content
and increase or set a greater margin
on the second key note button and let wrap your keys.
idea demo :
body {
margin: 0;
}
article {
display: flex;
flex-wrap: wrap;
justify-content: space-between;
}
div {
box-sizing: border-box;
border: solid;
padding: 1em;
margin: 1em 1vw 0;
width: 20%;
}
div:nth-child(2) {
margin: 1em 10% 0;
}
/* demo purpose */
div {
text-align: center;
}
div:nth-child(1) {
counter-reset: div 2;
}
div {
counter-increment: div;
}
div:nth-child(6) {
counter-reset: div;
}
div::before {
content: counters(div, ".", upper-alpha);
}
<article>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
</article>