ruby,refactoring,code-smell,guard-clause
What is wrong with this more explizit and more readable version? def my_method(x, y) error = validate(x, y) if error error else # do something else end end IMO there is no benefit in writing the shortest possible code. You should always aim to write the most readable and understandable...
python,django,django-views,code-smell,code-quality
The biggest problem is not the style of the code - it is that you are making 10 queries: 5 for getting the objects and 5 for updating them. Filter out objects using __in at once: @login_required def submission_set_rank(request): keys = {'rank1': 5, 'rank2': 4, 'rank3': 3, 'rank4': 2, 'rank5':...
PHP does support forward declaration of classes (as well as functions). The following code works fine on php 5: <?php class B extends A { public function __construct(){ echo A::property; } } class A { const property = "I am a const!".PHP_EOL; } $b = new B(); output: I am...
refactoring,code-smell,feature-envy
You described it pretty well. Inappropriate Intimacy means compromising the other class's encapsulation, such as by directly accessing instance variables that aren't meant to be directly accessed. Very bad. Fix the grabby class to only use public features of the compromised class and, if possible, change the compromised class so...
ruby,code-smell,context-injection
Looks like your Contact class serves two purposes - storing contact data and doing some authorized requests - so yes it does violate the Single Responsibility Principle. This could be fixed by splitting the Contact class into two - one maybe a Struct or even a plain hash to store...
unit-testing,refactoring,legacy,code-smell
I don't understand your comment about those two more techniques. You mock method because it is dependent on something you don't wan't in your test. You are not testing it. You are testing the rest of the code and this method (before it is mocked) prevents you from doing it....
java,oop,instanceof,code-smell
Following the invitation if the OP: If use of the interface has no other reason than to add a marker plus the update method to GEs, and if the type UGE isn't used except after this single instanceof, then it is a weak reason for having these extra types. ESpecially...
java,interface,immutability,code-smell,lsp
In my view, an ImmutableMap should implement Map. It would be a bad idea not to implement Map as there are many methods that accept a Map as an argument and only use it in a read-only sense. I don't believe this does violate the Liskov Subsitution principle because the...