Skip to main content
update formatting
Source Link
  1. I used the deferdefer attribute in my script tag and put it in the head.

    <script src="fizzbuzz.js" charset="utf-8" defer="defer"></script>
    
  1. I used the defer attribute in my script tag and put it in the head.

    <script src="fizzbuzz.js" charset="utf-8" defer="defer"></script>
    
  1. I used the defer attribute in my script tag and put it in the head.

    <script src="fizzbuzz.js" charset="utf-8" defer="defer"></script>
    
update formatting
Source Link

I discovered the FizzBuzz test while looking up something else on stackoverflow, so I decided to try it and see what I would come up with. I'm kind of new to jquery and jsJavaScript, so I'm looking for a critique on my code. What can I do better? Changes? Simplifications?

Here's a copy of the code, edited per st88's suggestion. The old calculateColumns() function is commented out, and has been replaced by two functions (createColumnsi.e. createColumns and calculateColumnscalculateColumns) that deal with the conditional differently and remove a repeating code error.

I discovered the FizzBuzz test while looking up something else on stackoverflow, so I decided to try it and see what I would come up with. I'm kind of new to jquery and js, so I'm looking for a critique on my code. What can I do better? Changes? Simplifications?

Here's a copy of the code, edited per st88's suggestion. The old calculateColumns() function is commented out, and has been replaced by two functions (createColumns and calculateColumns) that deal with the conditional differently and remove a repeating code error.

I discovered the FizzBuzz test while looking up something else on stackoverflow, so I decided to try it and see what I would come up with. I'm kind of new to jquery and JavaScript, so I'm looking for a critique on my code. What can I do better? Changes? Simplifications?

Here's a copy of the code, edited per st88's suggestion. The old calculateColumns() function is commented out, and has been replaced by two functions (i.e. createColumns and calculateColumns) that deal with the conditional differently and remove a repeating code error.

Tweeted twitter.com/StackCodeReview/status/731758504961282049
Updated the code per st88's suggestion.
Source Link

Here's a copy of the code:, edited per st88's suggestion. The old calculateColumns() function is commented out, and has been replaced by two functions (createColumns and calculateColumns) that deal with the conditional differently and remove a repeating code error.

var fizzContainer = $("div.number-container");

var divisor1 = 3,
  divisor2 = 5,
  maxFizz = 100,
  word1 = "Fizz",
  word2 = "Buzz",
  windowWidth = window.innerWidth,
  minNumOfColumns = 1,
  maxNumOfColumns = 12,
  widthPerColumn = 150,
  columns,
  linesPerColumn,
  columnSizingParameters = {};

calculateHeading(windowWidth);
calculateColumns();
createColumns();

$(window).resize(function() {
  windowWidth = window.innerWidth;
  calculateHeading(windowWidth);

  $(".number-container").replaceWith("<div class='number-container'></div>");
  calculateColumnscreateColumns();
});

function calculateHeading(windowSize) {
  if (windowSize <= 500) {
    $("h1").attr("class", "transition xSmallHeading");
  } else if (windowSize <= 700) {
    $("h1").attr("class", "transition smallHeading");
  } else if (windowSize <= 1000) {
    $("h1").attr("class", "transition mediumHeading");
  } else if (windowSize >= 1000) {
    $("h1").attr("class", "transition largeHeading");
  }
}


// function calculateColumns() {
//    for (var i = 1; i <= maxNumOfColumns; i++) {
//       if ((windowWidth >= columnSizingParameters[i]) && (windowWidth <= columnSizingParameters[i + 1])) {
//            columns = i;
//            break;
//       }
//    }
//    linesPerColumn = Math.ceil(maxFizz / columns);
//    createNumbers();
// }

function calculateColumns() {
  var columnIt = minNumOfColumns;minNumOfColumns - 1;
  for (columnIt; columnIt <= maxNumOfColumns; columnIt++) {
    columnSizingParameters[columnIt] = columnIt * widthPerColumn;
  }
}

function createColumns() {
  for (var i = 1; i <= maxNumOfColumns; i++++i) {
    if ((windowWidth >= columnSizingParameters[i]) &&> (windowWidth <= columnSizingParameters[i + 1])) {
      columns = i;i - 1;
      break;
    }
  }
  linesPerColumn = Math.ceil(maxFizz / columns);
  createNumbers();
}

function createNumbers() {
  var i = 1;

  for (var c = 1; c <= columns; c++) {
    var col = $('<div></div').appendTo(".number-container");
    col.addClass('column');

    for (var a = 1; a <= linesPerColumn && i <= maxFizz; a++, i++) {
      if ((i % divisor1 === 0) && (i % divisor2 === 0)) {
        createContentLine(word1 + word2, "double-match", col, i, true);
        continue;
      } else if (i % divisor1 === 0) {
        createContentLine(word1, "match-1", col, i, true);
        continue;
      } else if (i % divisor2 === 0) {
        createContentLine(word2, "match-2", col, i, true);
        continue;
      } else {
        createContentLine(i, "num", col, i, false);
        continue;
      }
    }
  }
}

function createContentLine(toPrint, cssClass, col, i, appendNumToWord) {
  var contentLine = $("<p>" + toPrint + "</p>").appendTo(col);
  contentLine.addClass(cssClass);

  if (appendNumToWord) {
    var appendedNumber = $("<span></span").appendTo(contentLine);
    appendedNumber.text(" • " + i).addClass("num");
  }
}
* {
 
  padding: 0px;
 
  margin: 0px;
 
  box-sizing: border-box;
 
}
 
.container {
 
  width: auto;
 
  height: 100%;
 
  display: block;
 
}
 
h1 {
 
  display: block;
 
  background-color: black;
 
  color: white;
 
  width: 100%;
 
  font-family: 'Berkshire Swash', cursive;
 
}
 
.transition {
 
  transition: 500ms;
 
}
 
/* Screen width Less than 500px */

.xSmallHeading {
 
  font-size: 25px;
 
  line-height: 25px;
 
  padding: 15px;
 
}
 
/* Less than 700px */

.smallHeading {
 
  font-size: 32px;
 
  line-height: 32px;
 
  padding: 20px;
 
}
 
/* Less than 1000px */

.mediumHeading {
 
  font-size: 40px;
 
  line-height: 40px;
 
  padding: 30px;
 
  margin-bottom: 10px;
 
}
 
/* Greater than 1000px */

.largeHeading {
 
  font-size: 50px;
 
  line-height: 50px;
 
  padding: 40px;
 
  margin-bottom: 20px;
 
}
 
.number-container {
 
  display: flex;
 
  margin-top: 20px;
 
  margin-bottom: 20px;
 
  text-align: center;
 
  line-height: 30px;
 
  width: 100%;
 
  height: calc(100% - 150px);
 
}
 
.column {
 
  flex-grow: 1;
 
  display: inline-block;
 
  border-right: 1px solid lightblue;
 
}
 
.column:last-of-type {
 
  border-right: none;
 
}
 
.num {
 
  color: #585858;
 
  font-weight: bold;
 
  font-style: normal;
 
  font-family: sans-serif;
 
  font-size: 12px;
 
}
 
.double-match {
 
  font-weight: 600;
 
  font-size: 20px;
 
  color: #3f89d6;
 
  font-family: 'Crimson Text', serif;
 
}
 
.match-1,
 
.match-2 {
 
  font-family: 'Josefin Slab', serif;
 
  font-weight: 700;
 
  font-size: 18px;
 
}
 
.match-1 {
 
  color: #7ac962;
 
}
 
.match-2 {
 
  color: #ba62d4
 
}
<html>

<head>
  <meta charset="utf-8">
  <title>FizzBuzz in Javascript</title>

  <link href='https://fonts.googleapis.com/css?family=Berkshire+Swash|Crimson+Text:600|Josefin+Slab:700' rel='stylesheet' type='text/css'>

  <link rel="stylesheet" href="style.css" media="screen" charset="utf-8">

  <script src="https://code.jquery.com/jquery-2.2.3.min.js" integrity="sha256-a23g1Nt4dtEYOj7bR+vTu7+T8VP13humZFBJNIYoEJo=" crossorigin="anonymous"></script>

  <script src="fizzbuzz.js" charset="utf-8" defer="defer"></script>
</head>

<body>
  <div class="container">

    <h1>The fizzbuzz test...in Javascript!</h1>

    <div class="number-container">

    </div>
  </div>
</body>

</html>

Here's a copy of the code:

var fizzContainer = $("div.number-container");

var divisor1 = 3,
  divisor2 = 5,
  maxFizz = 100,
  word1 = "Fizz",
  word2 = "Buzz",
  windowWidth = window.innerWidth,
  minNumOfColumns = 1,
  maxNumOfColumns = 12,
  widthPerColumn = 150,
  columns,
  linesPerColumn,
  columnSizingParameters = {};

calculateHeading(windowWidth);
calculateColumns();

$(window).resize(function() {
  windowWidth = window.innerWidth;
  calculateHeading(windowWidth);

  $(".number-container").replaceWith("<div class='number-container'></div>");
  calculateColumns();
});

function calculateHeading(windowSize) {
  if (windowSize <= 500) {
    $("h1").attr("class", "transition xSmallHeading");
  } else if (windowSize <= 700) {
    $("h1").attr("class", "transition smallHeading");
  } else if (windowSize <= 1000) {
    $("h1").attr("class", "transition mediumHeading");
  } else if (windowSize >= 1000) {
    $("h1").attr("class", "transition largeHeading");
  }
}

function calculateColumns() {
  var columnIt = minNumOfColumns;
  for (columnIt; columnIt <= maxNumOfColumns; columnIt++) {
    columnSizingParameters[columnIt] = columnIt * widthPerColumn;
  }

  for (var i = 1; i <= maxNumOfColumns; i++) {
    if ((windowWidth >= columnSizingParameters[i]) && (windowWidth <= columnSizingParameters[i + 1])) {
      columns = i;
    }
  }
  linesPerColumn = Math.ceil(maxFizz / columns);
  createNumbers();
}

function createNumbers() {
  var i = 1;

  for (var c = 1; c <= columns; c++) {
    var col = $('<div></div').appendTo(".number-container");
    col.addClass('column');

    for (var a = 1; a <= linesPerColumn && i <= maxFizz; a++, i++) {
      if ((i % divisor1 === 0) && (i % divisor2 === 0)) {
        createContentLine(word1 + word2, "double-match", col, i, true);
        continue;
      } else if (i % divisor1 === 0) {
        createContentLine(word1, "match-1", col, i, true);
        continue;
      } else if (i % divisor2 === 0) {
        createContentLine(word2, "match-2", col, i, true);
        continue;
      } else {
        createContentLine(i, "num", col, i, false);
        continue;
      }
    }
  }
}

function createContentLine(toPrint, cssClass, col, i, appendNumToWord) {
  var contentLine = $("<p>" + toPrint + "</p>").appendTo(col);
  contentLine.addClass(cssClass);

  if (appendNumToWord) {
    var appendedNumber = $("<span></span").appendTo(contentLine);
    appendedNumber.text(" • " + i).addClass("num");
  }
}
* {
 
  padding: 0px;
 
  margin: 0px;
 
  box-sizing: border-box;
 
}
 
.container {
 
  width: auto;
 
  height: 100%;
 
  display: block;
 
}
 
h1 {
 
  display: block;
 
  background-color: black;
 
  color: white;
 
  width: 100%;
 
  font-family: 'Berkshire Swash', cursive;
 
}
 
.transition {
 
  transition: 500ms;
 
}
 
/* Screen width Less than 500px */

.xSmallHeading {
 
  font-size: 25px;
 
  line-height: 25px;
 
  padding: 15px;
 
}
 
/* Less than 700px */

.smallHeading {
 
  font-size: 32px;
 
  line-height: 32px;
 
  padding: 20px;
 
}
 
/* Less than 1000px */

.mediumHeading {
 
  font-size: 40px;
 
  line-height: 40px;
 
  padding: 30px;
 
  margin-bottom: 10px;
 
}
 
/* Greater than 1000px */

.largeHeading {
 
  font-size: 50px;
 
  line-height: 50px;
 
  padding: 40px;
 
  margin-bottom: 20px;
 
}
 
.number-container {
 
  display: flex;
 
  margin-top: 20px;
 
  margin-bottom: 20px;
 
  text-align: center;
 
  line-height: 30px;
 
  width: 100%;
 
  height: calc(100% - 150px);
 
}
 
.column {
 
  flex-grow: 1;
 
  display: inline-block;
 
  border-right: 1px solid lightblue;
 
}
 
.column:last-of-type {
 
  border-right: none;
 
}
 
.num {
 
  color: #585858;
 
  font-weight: bold;
 
  font-style: normal;
 
  font-family: sans-serif;
 
  font-size: 12px;
 
}
 
.double-match {
 
  font-weight: 600;
 
  font-size: 20px;
 
  color: #3f89d6;
 
  font-family: 'Crimson Text', serif;
 
}
 
.match-1,
 
.match-2 {
 
  font-family: 'Josefin Slab', serif;
 
  font-weight: 700;
 
  font-size: 18px;
 
}
 
.match-1 {
 
  color: #7ac962;
 
}
 
.match-2 {
 
  color: #ba62d4
 
}
<html>

<head>
  <meta charset="utf-8">
  <title>FizzBuzz in Javascript</title>

  <link href='https://fonts.googleapis.com/css?family=Berkshire+Swash|Crimson+Text:600|Josefin+Slab:700' rel='stylesheet' type='text/css'>

  <link rel="stylesheet" href="style.css" media="screen" charset="utf-8">

  <script src="https://code.jquery.com/jquery-2.2.3.min.js" integrity="sha256-a23g1Nt4dtEYOj7bR+vTu7+T8VP13humZFBJNIYoEJo=" crossorigin="anonymous"></script>

  <script src="fizzbuzz.js" charset="utf-8" defer="defer"></script>
</head>

<body>
  <div class="container">

    <h1>The fizzbuzz test...in Javascript!</h1>

    <div class="number-container">

    </div>
  </div>
</body>

</html>

Here's a copy of the code, edited per st88's suggestion. The old calculateColumns() function is commented out, and has been replaced by two functions (createColumns and calculateColumns) that deal with the conditional differently and remove a repeating code error.

var fizzContainer = $("div.number-container");

var divisor1 = 3,
  divisor2 = 5,
  maxFizz = 100,
  word1 = "Fizz",
  word2 = "Buzz",
  windowWidth = window.innerWidth,
  minNumOfColumns = 1,
  maxNumOfColumns = 12,
  widthPerColumn = 150,
  columns,
  linesPerColumn,
  columnSizingParameters = {};

calculateHeading(windowWidth);
calculateColumns();
createColumns();

$(window).resize(function() {
  windowWidth = window.innerWidth;
  calculateHeading(windowWidth);

  $(".number-container").replaceWith("<div class='number-container'></div>");
  createColumns();
});

function calculateHeading(windowSize) {
  if (windowSize <= 500) {
    $("h1").attr("class", "transition xSmallHeading");
  } else if (windowSize <= 700) {
    $("h1").attr("class", "transition smallHeading");
  } else if (windowSize <= 1000) {
    $("h1").attr("class", "transition mediumHeading");
  } else if (windowSize >= 1000) {
    $("h1").attr("class", "transition largeHeading");
  }
}


// function calculateColumns() {
//    for (var i = 1; i <= maxNumOfColumns; i++) {
//       if ((windowWidth >= columnSizingParameters[i]) && (windowWidth <= columnSizingParameters[i + 1])) {
//            columns = i;
//            break;
//       }
//    }
//    linesPerColumn = Math.ceil(maxFizz / columns);
//    createNumbers();
// }

function calculateColumns() {
  var columnIt = minNumOfColumns - 1;
  for (columnIt; columnIt <= maxNumOfColumns; columnIt++) {
    columnSizingParameters[columnIt] = columnIt * widthPerColumn;
  }
}

function createColumns() {
  for (var i = 1; i <= maxNumOfColumns; ++i) {
    if (columnSizingParameters[i] > windowWidth) {
      columns = i - 1;
      break;
    }
  }
  linesPerColumn = Math.ceil(maxFizz / columns);
  createNumbers();
}

function createNumbers() {
  var i = 1;

  for (var c = 1; c <= columns; c++) {
    var col = $('<div></div').appendTo(".number-container");
    col.addClass('column');

    for (var a = 1; a <= linesPerColumn && i <= maxFizz; a++, i++) {
      if ((i % divisor1 === 0) && (i % divisor2 === 0)) {
        createContentLine(word1 + word2, "double-match", col, i, true);
        continue;
      } else if (i % divisor1 === 0) {
        createContentLine(word1, "match-1", col, i, true);
        continue;
      } else if (i % divisor2 === 0) {
        createContentLine(word2, "match-2", col, i, true);
        continue;
      } else {
        createContentLine(i, "num", col, i, false);
        continue;
      }
    }
  }
}

function createContentLine(toPrint, cssClass, col, i, appendNumToWord) {
  var contentLine = $("<p>" + toPrint + "</p>").appendTo(col);
  contentLine.addClass(cssClass);

  if (appendNumToWord) {
    var appendedNumber = $("<span></span").appendTo(contentLine);
    appendedNumber.text(" • " + i).addClass("num");
  }
}
* {
  padding: 0px;
  margin: 0px;
  box-sizing: border-box;
}
.container {
  width: auto;
  height: 100%;
  display: block;
}
h1 {
  display: block;
  background-color: black;
  color: white;
  width: 100%;
  font-family: 'Berkshire Swash', cursive;
}
.transition {
  transition: 500ms;
}
/* Screen width Less than 500px */

.xSmallHeading {
  font-size: 25px;
  line-height: 25px;
  padding: 15px;
}
/* Less than 700px */

.smallHeading {
  font-size: 32px;
  line-height: 32px;
  padding: 20px;
}
/* Less than 1000px */

.mediumHeading {
  font-size: 40px;
  line-height: 40px;
  padding: 30px;
  margin-bottom: 10px;
}
/* Greater than 1000px */

.largeHeading {
  font-size: 50px;
  line-height: 50px;
  padding: 40px;
  margin-bottom: 20px;
}
.number-container {
  display: flex;
  margin-top: 20px;
  margin-bottom: 20px;
  text-align: center;
  line-height: 30px;
  width: 100%;
  height: calc(100% - 150px);
}
.column {
  flex-grow: 1;
  display: inline-block;
  border-right: 1px solid lightblue;
}
.column:last-of-type {
  border-right: none;
}
.num {
  color: #585858;
  font-weight: bold;
  font-style: normal;
  font-family: sans-serif;
  font-size: 12px;
}
.double-match {
  font-weight: 600;
  font-size: 20px;
  color: #3f89d6;
  font-family: 'Crimson Text', serif;
}
.match-1,
.match-2 {
  font-family: 'Josefin Slab', serif;
  font-weight: 700;
  font-size: 18px;
}
.match-1 {
  color: #7ac962;
}
.match-2 {
  color: #ba62d4
}
<html>

<head>
  <meta charset="utf-8">
  <title>FizzBuzz in Javascript</title>

  <link href='https://fonts.googleapis.com/css?family=Berkshire+Swash|Crimson+Text:600|Josefin+Slab:700' rel='stylesheet' type='text/css'>

  <link rel="stylesheet" href="style.css" media="screen" charset="utf-8">

  <script src="https://code.jquery.com/jquery-2.2.3.min.js" integrity="sha256-a23g1Nt4dtEYOj7bR+vTu7+T8VP13humZFBJNIYoEJo=" crossorigin="anonymous"></script>

  <script src="fizzbuzz.js" charset="utf-8" defer="defer"></script>
</head>

<body>
  <div class="container">

    <h1>The fizzbuzz test...in Javascript!</h1>

    <div class="number-container">

    </div>
  </div>
</body>

</html>
Added a question about using an interval.
Source Link
Loading
deleted 26 characters in body
Source Link
200_success
  • 145.6k
  • 22
  • 191
  • 481
Loading
Reformatted as a code snippet per answer suggestion (thanks!).
Source Link
Loading
Source Link
Loading