Skip to content

final#227

Open
Backuphero wants to merge 1 commit intoprojectshft:masterfrom
Backuphero:master
Open

final#227
Backuphero wants to merge 1 commit intoprojectshft:masterfrom
Backuphero:master

Conversation

@Backuphero
Copy link

No description provided.

@@ -0,0 +1,54 @@
.comments-section {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many of the classes here you can achieve with bootstrap like m-3 for margins or btn-block

const postButton = document.getElementById('submit');
const messageBoard = document.getElementById('posts'); // Container for posts

var messages = [];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why var and not const or let? avoid var

var messages = [];

// Function to display messages dynamically
function postMessages() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next eval only es6!

// Function to create a message element
function createMessageElement(message, index) {
var div = document.createElement('div');
div.className = 'message';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be a const on top of the file and then you use it.

}

// Function to create a message element
function createMessageElement(message, index) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is huge and very hard to follow, I ended up forgetting Im inside of it while reading it. You have to extract the internal logic to helper functions in order to improve readability and maintainability with your code. Imagine that delete has a bug, will be hard to find here.

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

Successfully merging this pull request may close these issues.

2 participants

Comments