New answers tagged

1

As said by @TorbenPutkonen, if you have the ownership on your dao classes then you can parameterize them. abstract class Repository<X> { private LiveData<List<X>> allEntities; private Dao<X> dao; void insert(X entity); } abstract class ListingFragmentViewModel<X> { private Repository<X> repository; public ...


3

I'm going to assume this question is in the context of quality interview code, not quality production code. Websites like GeeksForGeeks (https://practice.geeksforgeeks.org/problems/jumping-caterpillars/0) don't grade you based on production code quality. Your failure to correctly spell the most important word in the problem domain (Caterpillar) would put ...


1

This code does in fact print the expected result of dog runs, so I'm going to go ahead and give it a quick review. First: your mixed method does not use the self parameter, so it could be a @staticmethod. Figuring out which of your object's methods actually depend on the object state makes it easier to figure out which methods might affect each other. ...


2

First of all, that's quite a robust approach from the architectural point of view. The only drawbacks are in the inconsistent implementation. 1. Autoload. Forget about require_once forever. use namespaces store your classes in catalogs the the same name as namespaces either call spl_autload_register() manually with a simple function that will require ...


2

I find relying on Sheet.Activate / Sheet.Deactivate to set/unset the sheetTables reference is rather frail, error-prone (miss an Activate event for whatever reason (Application.EnableEvents being toggled off, for example), and just like that the Change handler starts throwing error 91), and doesn't really make much sense: the table exists on Sheet1 as long ...


2

Inconsistent Function Imports Import all used functions or none. If I see this: use function array_key_exists; I assume that it is the only global function being used in that file. To my surprise, it is not true as password_hash is used inside the same file too. Either import them all, or none. I myself prefer no imports and instead prefix global ...


4

If Not DebugMode Then Resume CleanExit Else: Stop: Resume Avoid using the : instruction separator in code. That's good for golfing up quick throw-away code in the immediate pane, not for production code. Statement syntax is fine when the Then part is a single statement - if it gets any more complex than that, block syntax should be used for clarity and ...


1

Having a default value is a good idea. Also, instead of of using the constructor directly, there is a way to convert from a hash table. This way only works if the class has a default (no-argument) constructor. (If you define a constructor that takes several arguments, you must also explicitly define a no-argument constructor.) Enum PxLogType { BlankLine; ...


2

It's tradeoffs. On one hand you get useful static methods in a toolbox, functions that don't accidentally show up in Excel's IntelliSense in the formula bar. On the other hand, you could get the very same out of a standard module with Option Private Module specified, minus the possibility of client code mistakenly attempting to create a New instance of the ...


3

On the whole nice work! This is actually a problem I've looked at in the past, and as a result I'd like to review the approach you've taken and the API/ way users can interact with your code. As I understand it you have 3 main classes Tables - responsible for instantiating a collection of Table objects (with some automagic methods to allow client code to ...


0

The problem I have with both versions is that you are adding functionality from all different items into 1 place. The first red flag for me is Class Cart extends Shop { Does a Cart really extend a Shop? You also have functionality for an order, a product etc. inside the Cart. What I have started to do is to break this down into what can be tangible ...


2

Overall You look like a Java programmer that is coming over to C++. This: { StudentDatabase* database = new StudentDatabase(); ... STUFF delete database; } Is very bad practice. There is no need to dynamically allocate this object (it can easily be an automatic variable thus creation and destruction is done automatically). You don't ...


1

You should indeed use smart pointers. Consider what happens if you copy your database, which only copies the pointers to the students, then you delete the first database, which deletes the students and leaves you with dangling pointers. This is just bad design. Also: delete* iter is typographically weird, even though the interpretation by the compiler is ...


3

You don't need to use new everywhere. Inside your main function, the StudentDatabase could very well reside on the stack so you don't need to delete it yourself. One of the issues with how StudentDatabase is written now is ownership semantics. Once a student pointer is added to the database, the database asumes ownership upon it and deletes it in the ...


0

app.input(input); app.process(); var result = app.output(); This is a bad practice. Every function/method has means of passing input, it has body and it has return value. Setting the input to class field, then calling method without arguments to process the set input, then storing result to another class field, then calling another method to retrive the ...


0

You can easily check this for yourself. Just define a variable std::size_t activeStudents = 0 in Student.cpp, which you increase in the Student constructor and decrease in the destructor. If at the end of main this variable has the value 0, you're fine. Same for the StudentDatabase. This approach is only appropriate as long as you are learning about memory ...


1

There is a lot to unpack here, so this review will be in multiple edits. GUI Creation You are creating and storing some widgets unnecessarily in member variables. For instance, self.truelabel is only used in the TruePosition constructor, it could be a local variable instead of a member variable. The same is true for almost all of the other labels. Your ...


2

I am not a professional game developer, just trying to help you base on my experience with Ruby so far. Glad to see your willingness of coding correctly from the beginning. Some points to improve: 1.Move the bot names and game mode options to CONSTANTS 2.Prompting for user's input is duplicated many times, you should make a function that take prompting ...


3

The biggest thing that will allow the code to clean up a fair bit is if you had made a subclass of Player for AIPlayer: class AIPlayer < Player NAMES = ['Bob', 'Joseph', 'Catherine', 'Paul', 'Tom', 'Baker'] def initialize super NAMES.sample end end that will allow you to add a method get_weapon to Player, which would ask for the user input ...


3

If this design is for a school project, and you've already submitted it, I fear you will have already failed the assignment. This design is bad. Static As a guiding principle, Java programs should use the identifier static exactly once, specifically in the declaration public static void main(String args[]). Any other usages of static should sound warning ...


2

It's good to have single method to control flow as you have in B, but this method should be mostly calling other methods of your class (you also call there checkQuantity for example, which is also in A). That way add is still very flexible - you can override those methods (probably protected methods, not public), that it calls or as last resort override that ...


1

It is also applicable by using the strategy pattern. You can use the operations(+,-,/,*) by strategy. And you can give this strategy as a context to the calculate method. package OOPDesign.calculator; public interface CalculationStrategy { int calculate(int value1, int value2); } AdditionStrategy package OOPDesign.calculator; public class ...


1

Because this question is tagged with object-oriented, I will focus on some improvements based on the oop-design. Currently your code uses objects, but this does not mean that it is object-oriented. When we take a look to your code all methods are global. With objects we can couple the methods to an object. For example: The methods addBookToLibrary and ...


3

I agree, that your naming is mostly alright, no point to document these variables. Except for event variables. What I seriously don't like about your approach is how tightly it is bound to html. For example removeBook takes as parameter event, which you use to extract element and use that one to extract data that you use to remove. That is bad from many ...


2

You are probably looking for factory method pattern, instead of strategy pattern. The CustomItemSender class is a good indicator of this. Whenever you extend a class to only override the constructor, you probably need a factory instead. It means you need somewhat more complex way of constructing the object and that implies creational pattern. Strategy is a ...


4

Issues Sharon Ben Asher did not mention: Because you're not using the index variable i to anything other than indexing the array you should use the for-each variant of the for loop to make the code cleaner. LinkedList is the wrong data type. Since you're only adding and never modifying the list and know the size you should use an ArrayList with correct ...


5

Errors Modify variable inside lambda expression: You sure this code passes compilation? you have a lambda expression (forEach()) that modifies a variable. all variables that are used inside lambda expression are considered final. by the way, the whole calculation of totalAge can be converted to stream() processing using map() and sum() The switch statement: ...


6

All the methods in InvokeRequest are basically the same. The only part that differs is the URL. Just pass that in instead of passing in the parts of the URL and constructing the URL in the function. Before I show the function that I came up with though, I'll note that there's no reason InvokeRequest should be a class, and there's also no reason that self....


2

Specific suggestions: The try/catch blocks should be as small as possible. In the first case that means only bet = int(input('Enter the amount to bet ')) should be guarded, and in the second case only control_value = input("Enter your choice - (H)it or (S)tand ").upper(). You could still take this much further in terms of OO. Game and Dealer classes, for ...


6

I question if a hand is a deck of cards. Certain functionality that you have, like shuffling, viewing, and inserting to specific positions, doesn't seem to make much sense in the context of a hand. A hand may be a collections of cards, similar to a deck, but that doesn't mean one is the other. Also, I don't think having points as an attribute is ideal. That'...


6

Don't have much time, just some things I noticed. Deck.__str__ This method can utilize the .join built in method. Your current implementation leaves a "," at the end of the string. .join is smart enough not to. Take a look: def __str__(self): return ', '.join(str(card) for card in cards).strip() Does the exact same thing yours does, but in one line. ...


1

Due to time constraints, I have only viewed the first two classes, here are my comments: Main.java There is no need for a private default constructor, unless you explicitly want to prevent from calling that method (e.g. in a singleton design pattern) In my eyes, the whole class is redundant. the main() method can be placed in Calculator. it is common ...


3

You have just added an unneeded layer of complexity to the problem. You can just create the factory as an Object const factory = { select(opts, text) { return (<select options={opts} labelText={text} />) }, input(opts, text) { return (<input labelText={text} />) }, default() { return new Error("Value Not Found") } }; Then call the ...


4

The "spirit" of Object Oriented Programming is two-fold: To give us an enforceable metaphor for reasoning about our code. ("objects" as objects) To give us heuristics about how to compartmentalize out code. (encapsulation) The purist perspective of OOP can be a bit verbose, and in python it's not usually necessary. I want then a GUI with TKinter, a ...


4

From the OOP/typing level, you might want to consider having Disk be a type. If it doesn't need any methods, you could just make it a subtype of int: from typing import NewType Disk = NewType('Disk', int) Some notes on your interfaces: Since you always use get_last_disk and delete_disk together, why not combine them? The thing you really want is a ...


2

In addition to what @Carcigenicate said: Spell out color instead of col - I thought it was short for column. pygame has a Rect class. Use it instead of separate x,y,width,height attributes. The Rect class already implements is_over-functionality. If desired, some common code can be pulled out into a parent/base class. Make text bliting into a standalone ...


3

(I'm on my phone on a road trip, so I can't do anything fancy here) Your issues with roster are because you have it as a attribute of the class, not an instance attribute. That means that every instance shares the same roster. You need to define it in the __init__ (or elsewhere, but ideally in the initializer) as self.roster, just like you did with the ...


2

You're right that you shouldn't make assumptions about the DOM when writing your state manager. Actually you should start by writing just your state manager, without worrying how that state should be reflected in the DOM. When your state manager works, you can write a separate class that does the DOM manipulation - based on the state in your state manager ...


1

Use __init__ to handle bunch of arguments, then your self will take of passing arguments to your respective definition - that's the good way to use OOP in python. class Foo: def __init__(self, value_a, value_b): self.a = value_a self.b = value_b def func(self): pass To avoid multiple if else you can have list or dict and ...


Top 50 recent answers are included