31

I have since been informed that the team was hoping for "A more OO design"...felt like I took it in that direction, personally. All the methods in your DrinkMachine class are static. I believe the ability to have more than one DrinkMachine is a very important feature. DrinkMachines should be able to have different inventories, different products, etc. Now, ...


29

There's a lot that can be improved here, so I hope that these suggestions are useful to you. Don't abuse using namespace std Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. Make sure you have all required #includes The code uses rand() but doesn't #include <cstdlib>. It's important to make sure ...


23

Your main() function desperately needs to be busted up, for multiple reasons: You use a lot of variables, all of them declared at the top of main(). A human mind is only good at keeping track of about 7 things at a time, so this code is hard to follow. It violates the Single Responsibility Principle, by parsing the command line, throwing the dice, keeping ...


23

First of all, I'm sorry that you didn't get the job. Better luck next time :) Overall, I don't think your code is bad. But I do have a couple of points: OOP My main complaint would be that you are missing a recipe class. Having it as a string array/map is not very flexible. Because of this, you also expose the inner workings of the drink class to the ...


19

Just a quick comment: Your Ocean class knows/does too many things. I'd be expecting a Fish class and a Shark class. Creating an ocean requires a starveTime constructor parameter? That's a sign you've broken the single responsibility principle (SRP). Methods like void addFish(int x, int y) would be void addFish(Fish fish) - let the fish know where he is in ...


18

I agree with their assessment that the OOP design could be improved. In particular, the way you initialize the data is cumbersome, and I object to the necessity of calling updateMakeable() throughout your DrinkMachine code. Ingredient.java What makes an ingredient comparable to another? Is the comparison by name, cost, or quantity? Having the stock be a ...


18

I started with the Julia code you had and also got ~20 seconds, so I think my timings are similar to yours. Let me give a step by step breakdown on how to do this. To start, notice that if you are running code in the REPL that variables defined there are global. This incurs a good performance cost. There are two ways to deal with this: 1) Wrap it all in a ...


15

Line by line analysis Use four spaces to indent python code. Never use tabs. from random import randint from sys import exit import os It is a good practice to do top-level imports (without importing module members). This way it's easier to understand what package the methods come from. os.system('clear') print "Welcome to the dice rolling simulator!" ...


15

There are many, many things wrong with your code; I'll list some, and then I advise you to fix as many of them as you can (and more) and then repost a new question with the revised code. First of all, you should compile your code with -W -Wall -Wextra (or -W4 if you use MSVC). Read the first warning diagnostic. Fix it. Recompile. Read the first diagnostic. ...


14

Here are some observations that may help you improve your code. Make sure to #include all required headers This program calls printf and srand but does not include the corresponding headers. Fix that by adding these lines: #include <cstdio> #include <cstdlib> Use objects You have a Particle structure and then separate functions that operate ...


14

This looks good overall. For the code review, I'll start with general comments and then try to get into smaller details. Documentation Documentating the code looks like an easy task but doing it properly so that it actually adds interesting information without adding too much noise can be pretty hard. Let's see what could be improved here. For a start, ...


13

As per Java naming convention, user-defined types should be capitalized. In this case, diceGame should be DiceGame. Prefer to have variables defined in the lowest scope possible. This is especially useful in not having to worry about whether or not a variable is still in use. Since dice1 and dice2 are only used within the loop, you can just initialize ...


13

Animal private int SIZE; Why do you start using uppercase here? It's no constant. Animal(int xcoord, int ycoord, int t, int size) { x=xcoord; Alternatively you can name our field xcoord and the constructor's argument x. It's equally wrong. Use just x for both and this.x = x; This is the normal style and it's absolutely clear what argument ...


13

There are several things I would do to increase readability of your code. The first would be to create methods that query some field. It will allow the code to be more human readable and self documenting. So for example you would have methods such as: private boolean isOutOfPaper() { return amountOfPaper == 0; } private boolean isOutOfToner() { ...


13

1. Design I think that using the @mean_of_experiments decorator is not the best approach to this problem: There might be cases where you want to run the underlying function by itself, for example in order to test it, but the decorator prevents this. You have to choose the number of experiments, N, when you define the function. This is restrictive: it means ...


12

#!/usr/bin/python import random, ConfigParser, ast, copy from optparse import OptionParser # Global variables. g_config = {} # Stores GameType config options. g_prizes = {} # Stores prizes from config file. g_prize_amounts = {} # Stores prize amounts from config file. g_ticket_quick_picks = [] # Store an array of ...


12

1. Failed requirements The program fails to implement the following details from the specification: The specification says, "Input parameters are ..." but these parameters as implemented as global variables and not as inputs. One of the parameters is "number of engineers", but the program makes no use of this number; it does not even represent the ...


12

Extract repeated code into functions. Loading the tile images is the same operation with a different file name. def load_scaled_image(file_name): return pygame.transform.scale(pygame.image.load(file_name), (TILE_SIZE, TILE_SIZE)) VILLAGE = load_scaled_image("village.png") GRASS = load_scaled_image("grass.png") WATER = load_scaled_image("water.png") ...


12

After reading the program, I think it is pretty good for start, but there is some room for further improvement. Small improvements Imports Consider importing specific classes, instead of using the wildcard import, so that your namespace is not cluttered up. (Although there are also benefits in importing the whole package, see this SO question). Double ...


12

Very nice program for a beginner! Great attempt! Here are some points. Imports For multiple imports for the same module, you use , from x_module import y, z But for our purpose, import math import random works better. Python is concerned about readability and space saving might not always be the best option. Naming Python follows a style convention ...


11

200_success has suggested that you change the readability of the line: dice1 = 1 + (int) ((Math.random() * (6 - 1)) + 1); to be: 1 + (int)(6 * Math.random()); (but then continues to use your version in his subsequent code) This is a good suggestion, but, more than that, you have to do that (or something similar) because your version of the dice-throw ...


11

I would like to add one more thing, in while loop, the type conversion is happening on each iteration. while (count < Integer.parseInt(args[0])) { count = count + 1; It should be done only once: int rolls = Integer.parseInt(args[0]); while (count < rolls) { count = count + 1; Which would be better written as a for loop: int rolls = Integer....


11

The biggest issue with your code is, as you remarked, the use of global variables. Cosmetically, the code is quite readable, but there's still room for improvement: Try to keep line length under 100 characters. For some people even 80 is too much. Variable declarations are more readable when there is one variable per line: int m = 0, n = 0, a[22]; ...


10

AJ, I like it. Very nicely coded. Here are some comments. I've omitted things already mentioned by @LokiAstari forward declaration of struct Transition is unnecessary. die, prefer to print errors to stderr exit(1) prefer exit(EXIT_FAILURE) some noisey comments ("exit unsucessfully", "allocate memory" etc) State_create does not initialise transitions (...


10

For starters, code as written won't compile in GCC. :-( "void main() is explicitly prohibited by the C++ standard and shouldn't be used" https://stackoverflow.com/questions/204476/what-should-main-return-in-c-c Rather than #include <time.h>, #include <cstdlib>. That will correctly give you rand and abs. There used to be some flak about ...


10

Some improvements: Use of refresh() as a function used to generate the number values, which generate the values AND writes into the DOM element. Use of a function to manage all user messages, write_message() Remove of all the old fashion HTML tags, and change it for CSS. A function for managing spending the "credits" to make it easier to manage them. &...


10

@Nobody has already commented on a number of issues at the level of individual lines of code. While these changes would help the code make a little better use of C++, most of them apply about equally to C. To make noticeably better use of C++, you probably want to look at the basic structure of the program. It seems reasonable to me that a "simulation" ...


10

This cleanup code at the end is indicative of the problem with this program: clear i clear j clear k clear l clear m clear n clear o clear p Nobody is going to reverse-engineer your minified / obfuscated code to understand it. You will probably have a hard time understanding it yourself if you come back to it after a few weeks. The program is therefore ...


10

As mentioned in the comments, there's not much to review. Don't abuse using namespace std Putting using namespace std within your program is generally a bad habit that you'd do well to avoid. Factor out common code In this case, the code gets a lock, does something, and releases the lock. That could easily be isolated to a common function: class ...


Only top voted, non community-wiki answers of a minimum length are eligible