enterprisequalitycoding / fizzbuzzenterpriseedition Goto Github PK

To make sure stability and security are on high enterprise standard, you should use at least 6 years old version of java.

Hello, this is an automated style guide report from when I tried to check in at my company. I'm a little confused what these methods are supposed to do, and I'm thankful that I have my hands held as I try to do my job.

///////////////////////////
You are missing required comments on the following methods:

class BuzzStrategy
public boolean isEvenlyDivisible(int i)

class FizzStrategy
public boolean isEvenlyDivisible(int i)

class IsEvenlyDivisibleStrategy
public boolean isEvenlyDivisible(int i)

and 145 other instances.

As a reminder, it's important to comment all public methods for clarity and collaboration.

Need to test over glassfish, jboss and websphere
http://arquillian.org/

I noticed that you don't test against Integer.MIN_VALUE, Integer.MAX_VALUE, 0, or -1.

I think we want to throw an error in everything but MAX_VALUE.

Testing for Integer.MAX_VALUE may be a bit difficult.

I've found a Perl script that can generate them for you: https://gist.github.com/sugyan/339237 , we'd have to run it during the setup phase with something like this: http://stackoverflow.com/a/8008156/42082

Maybe we can get it to run as a scheduled job during the build process.

This starts to beg the question of if we should be using log4j or some other logger to monitor the output of the program.

Assume it's tied up in legal.

Enterprise app should have UML diagrams reviewed and approved by architects.

HAS NIST FIPS PUB 199 COMPLIANCE BEEN ACHIEVED?

The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.

It currently relies on zero. It needs to rely on at least two more to justify our budget numbers.

Add Maven and package as an EAR to show that the code absolutely enterprise-ready to be deployed into a high-speed, high-volume enterprise environment.

On README.md

Last section reads:

For numbers 1 through 100,

if the number is divisible by 3 print Fizz;
if the number is divisilbe by 5 print Buzz;
if the number is divisible by 3 and 5 (15) print FizzBuzz;
else, print the number.

It should read:

For numbers 1 through 100,

if the number is divisible by 3 print Fizz;
if the number is divisible by 5 print Buzz;
if the number is divisible by 3 and 5 (15) print FizzBuzz;
else, print the number.

SAML 2.0 is at a minimun. Consider WS-Security as well.

If you remember from our meeting with the client in december, They expect to have 10,000 concurrent users of Fizzbuzz at any given time. I think we need to support up to 100,000 just in case.

Mary, can you come up with a quote from the guys at Loadrunner to see what it will take to get the boxes and software we need for this? And how much we need to bill the client for this?

Here https://github.com/Mikkeren/FizzBuzzEnterpriseEdition/blob/master/src/main/java/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/strategies/NoFizzNoBuzzStrategy.java and here https://github.com/Mikkeren/FizzBuzzEnterpriseEdition/blob/master/src/main/java/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/strategies/BuzzStrategy.java and here https://github.com/Mikkeren/FizzBuzzEnterpriseEdition/blob/master/src/main/java/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/strategies/FizzStrategy.java

there's code that goes like this

if ((((int)(i / FizzStrategyConstants.FIZZ_INTEGER_CONSTANT_VALUE)) * FizzStrategyConstants.FIZZ_INTEGER_CONSTANT_VALUE == i)) {
return true;
} else {
return false;
}

Now that cleverly engineered code for checking (a%b==0) is in effect the same everywhere, just the constant changes. In the meantime there's a chance that this approach to checking (a%b==0) happens to be buggy and needs to be enhanced (perhaps by adding an enterprise version of a crystal ball) - then it needs to be changed everywhere. So that's a program maintenance and extensibility showstopper.

Code for checking (a%b==0) needs to be factored out into some kind of reusable mechanism.

Please add branches 1999.1-dev, 1999.2-dev, etc. (through to our 2012.2 release) - our offshore development excellence team will be using these.

Also: missing document describing the 13-way merge process for getting patches on legacy code into master.

Where is the tagging schedule? Did you update the sharepoint?

This repository is hosted on Github, which is fine for individuals who want to play around but is inappropriate for serious businesses. Please move this repository to Perforce.

1. Introduce interfaces ReturnableAsString with method String getAsString()
2. Introduce StringReturner with method <T extends ReturnableAsString> getReturnString(T returnFrom)
3. Implement StringReturnableAsString implement ReturnableAsString
4. Implement IntReturnableString implement ReturnableAsString
5. Introduce StringReturnerImpl

FizzBuzz is not a useful standalone library if it always prints to standard out. Worse, redirecting standard out for a test is hacky and precludes the use of a concurrent test framework.

You should pass a reference to the output stream directly into FizzBuzz from main() and down into the individual printers.

Log4J, or implement a custom enterprise logging framework. Log to database. MongoDB?

The range is [1;99], not [1;100]. There's probably a < 100 that should be <= 100.

[Redacted]

Instead of using < and <= for comparison, a comparator method should be used. In addition to being more explicit, it will make unit testing much easier, especially once the project transitions over to using Spring Dependency Injection.

Instead of

if (myFizzStrategy.isEvenlyDivisible(i)) { 
    myFizzStringPrinter.print();
}
if (myBuzzStrategy.isEvenlyDivisible(i)) { 
    myBuzzStringPrinter.print();
}
if (myNoFizzNoBuzzStrategy.isEvenlyDivisible(i)) {
    myIntIntegerPrinter.printInteger(i);
}
myNewLinePrinter.print();

You could create a composite containing myFizzStrategy and myBuzzStrategy. You could also create an adapter interface for the myIntIntegerPrinter to throw it in there too.

You'll probably have to write a rules engine for constructing your composite in the right order. I'm not sure how you can make sure 3 always comes before 5, but I'm sure you can find some code on stackoverflow. This shouldn't take more than a few weeks, a month tops.

Once you have you composite you can just call print on it and this long, nasty branching goes away.

What if some part of functionality needs to be changed in runtime? OSGI should be used.

To truly be an enterprise product I'll need to see a support contract in case it Fizzes when it was supposed to Buzz.

It will run after goal test to fizzbuzz failed tests.

What if we want web-scale with fizz and buzz?

function com.seriouscompany.business.java.fizzbuzz.packagenamingpackage.impl.FizzBuzz.fizzbuzz(int n) is too big to understand.
Also I think that name n for an argument is too simple and it doesn't reveal its real meaning.

Hi, this is a great app but unfortunately we use Microsoft technologies in-house so I have been forced to create a .NET version. It can be found here:

https://github.com/jongeorge1/FizzBuzzEnterpriseEdition-CSharp

Please note that I may be forced to remove this soon - I have been informed that we only use Visual Basic.NET and that I should have used this rather than C#. I have filled in the appropriate paperwork to request an exception in this case, and it is currently being reviewed by management.

Currently there is no testing plan. Our QA department is blocked until this is ready.

This is ridiculous - if I want to have another implementation of FizzBuzz I have to change main() because class FizzBuzz is hardcoded in there. The code is therefore not extensible which won't do for a SeriousBusiness. Class FizzBuzz should be implementing an appropriate interface and be created through a factory.

Please introduce Hadoop. Or your own mr framework (better)

This application does not meet expected performance goals. A careful evaluation of threading strategy for parallel computation of calculations will likely result in action items which, when implemented, will enhance performance to acceptable levels.

In https://github.com/Mikkeren/FizzBuzzEnterpriseEdition/blob/be96d3868ec0df7d08d3dc153df41055eceabcaf/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/Main.java the class com.seriouscompany.business.java.fizzbuzz.packagenamingpackage.impl.FizzBuzz is instantiated with FizzBuzz myFizzBuzz = new FizzBuzz();. Clearly, if the requester were to ask for other problems to be solved, this solution would not scale. A ProblemFactory should be able to instantiate a Problem instance by name, allowing other problems to be solved by passing a command line argument (or reading from a configuration file).

That's what this is for right?

And follower the producer consumer pattern.

This library is not leveraging aspect oriented programming to implement cross cutting concerns. Please implement AOP audit logging so I can use this library in production.

Thank you

In https://github.com/Mikkeren/FizzBuzzEnterpriseEdition/blob/65bbb7b88c0f8b27b326936ff296bf5b1a2a3568/src/main/java/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/strategies/constants/NoFizzNoBuzzStrategyConstants.java there're two newlines in row - lines 2 and 3. How could this have happened in an enterprise grade Serious Project?

Can be implemented via a web service framework.

http://en.wikipedia.org/wiki/List_of_web_service_frameworks

Use Java EE annotations like @entity, @service, @repository etc to integrate into Java EE containers and DI

No serious enterprise application will have a main() method. This bug is related to the missing DI bug. Perhaps Spring? The pom bug could help us resolve all of these issues!

As a FizzBuzz user, I'd like to print different strings in our dev, test, perf test, staging, pre-prod, prod, and legacy FizzBuzz environments.

Don't know if I did this right, Johns out and he usually adds things for me.

-bob

BuzzStrategy, FizzStrategy and NoFizzNoBuzzStrategy have several problems:

1. BuzzStrategy and FizzStrategy are two instances of same class; only difference is an integer value - should be refactored to inherit from a SomethingZzStrategy class.
2. SomethingZzStrategy should received integer value NOT as a hard-coded value in BuzzFactory and FizzFactory. Current implementation states:
   if ((((int)(i / 5)) * 5 == i)) {
   and is obviously problematic as there are two different places of the hard coded value. If a coder changes one 5 and not the other, the class will break. This is just asking for trouble.
3. Integer value used in SomethingZzStrategy should be read and stored in a configuration file, to allow end-users to configure the tool.
4. NoFizzNoBuzzStrategy should use already implemented behavior of BuzzFactory and FizzFacotry to implement it - smart code reuse

Not acceptable without proper documentation.

Configuring enterprise-class applications in code is brittle and leads to maintenance headaches. At the very minimum, the number 100 should be factored out to an XML configuration file. Possible structure:

<app>
  <name>FizzBuzz</ name>
  <config>
    <name>numberOfLoops</name>
    <value type="int">100</value>
  </config>
</app>

I am concerned that this structure might not be general enough. Suggestions welcome for ways to improve it.

However, merely putting configuration into XML is not enough. Truly enterprise class applications use XML to wire together the enterprise bean objects dynamically, so as not to be constrained by the arbitrary limitations of source-level programming.