HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

FizzBuzz-type program that reads/writes to Excel

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
writesexcelprogramfizzbuzztypereadsthat

Problem

I'm new to Groovy, and coming from PHP it's been a rough transition.

The code below works. However, I would appreciate feedback on how better to write it and follow good practice with Groovy.

I anticipate doing a lot of Excel work in the future, and so tried to make this program reusable for more than just FizzBuzz.

```
/*
* In Excel: Create a workbook with a blank sheet, "output"
* and a sheet "input" with the values
* startValue : 0
* endValue : 100
* fizz : 3
* buzz : 5
*
* In Groovy: Create a program that will
* - Open an Excel file
* - Extract the four parameters above
* - Run a "FizzBuzz" on those parameters
* - Write the results to the "output" sheet
*
* FizzBuzz: For all values in a range
* If the value is divisible by 3, print "fizz"
* If the value is divisible by 5, print "buzz"
* If the value is divisible by 3 and 5, print "fizzbuzz"
* Otherwise print the value
*
* The program below uses Apache POI to parse Excel.
*/

package org.example

import java.io.FileNotFoundException
import java.io.FileOutputStream
import java.io.IOException

import org.apache.poi.ss.usermodel.*
import org.apache.poi.xssf.usermodel.*

class Excel {

def file;
def workbook;

static main(def args) throws Exception {
new Excel()
}

Excel(){
file = new FileInputStream("input.xlsx")
workbook = WorkbookFactory.create(file)
def sheetIn = workbook.getSheet("input")
def fizzbuzz = new Excel.FizzBuzz()
def params = fizzbuzz.setFromSheet(sheetIn)
def sheetOut = workbook.getSheet("output")

this.doByRow(sheetOut, params.start, params.end, { rowNum, value ->
def row = row(sheetOut, rowNum)
setCell(row, 0, value)
setCell(row, 1, fizzbuzz.calculate(value))
})
def fileOut = new FileOutputStream(filename("fizzbuzz"))
workbook.write(fileOut)
}

def filename(value){
return "out/" + value + "_" +

Solution

Why would FizzBuzz need to know about Excel at all?

class FizzBuzz{
    def params = [
        "start" :0,
        "end"   :0,
        "fizz"  :0,
        "buzz"  :0
        ]

    def setFromSheet(sheet){
        this.params.start   = Excel.this.hlookup(sheet,"startValue",1)
        this.params.end     = Excel.this.hlookup(sheet,"endValue",1)
        this.params.fizz    = Excel.this.hlookup(sheet,"fizz",1)
        this.params.buzz    = Excel.this.hlookup(sheet,"buzz",1)
        return this.params;
    }


As it is, you couldn't take your FizzBuzz class and use it in a different context. Your FizzBuzz implementation shouldn't depend on Excel. It should take some values in, do some work on them, and return them in a new form. Your Excel class (bad name by the way) should be responsible for fetching those values and passing them to FizzBuzz. I would recommend making its constructor take in start, end, fizz, and buzz as arguments instead. This breaks the dependency and makes it so you could use the class in a console program (or anywhere really) if you so chose.

Code Snippets

class FizzBuzz{
    def params = [
        "start" :0,
        "end"   :0,
        "fizz"  :0,
        "buzz"  :0
        ]

    def setFromSheet(sheet){
        this.params.start   = Excel.this.hlookup(sheet,"startValue",1)
        this.params.end     = Excel.this.hlookup(sheet,"endValue",1)
        this.params.fizz    = Excel.this.hlookup(sheet,"fizz",1)
        this.params.buzz    = Excel.this.hlookup(sheet,"buzz",1)
        return this.params;
    }

Context

StackExchange Code Review Q#77419, answer score: 4

Revisions (0)

No revisions yet.