Friday, December 15, 2017

Code Smell



https://dzone.com/articles/shotgun-surgery-what-it-is-and-how-to-stop-it

https://sourcemaking.com/refactoring/smells
Bloaters
Bloaters are code, methods and classes that have increased to such gargantuan proportions that they are hard to work with. Usually these smells do not crop up right away, rather they accumulate over time as the program evolves (and especially when nobody makes an effort to eradicate them).
Long Method
Large Class
Primitive Obsession
Long Parameter List
Data Clumps
https://martinfowler.com/bliki/DataClump.html
Whenever two or three values are gathered together - turn them into a $%#$%^ object.
This is one of my favorite CodeSmells from the refactoring book. You spot it when you constantly see the same few data items passed around together. start and end are a good example of a data clump wanting to be a range. Often data clumps are primitive values that nobody thinks to turn into an object.
The first step is to replace data clumps with objects and use the objects whenever you see them. An immediate benefit is that you'll shrink some parameter lists. The interesting stuff happens as you begin to look for behavior to move into the new objects.
  • If some of the data is passed to other methods, think about passing the entire data object to the method instead of just individual fields. Preserve Whole Object will help with this.
https://sourcemaking.com/refactoring/preserve-whole-object

Object-Orientation Abusers
All these smells are incomplete or incorrect application of object-oriented programming principles.

Switch Statements
https://sourcemaking.com/refactoring/smells/switch-statements
You have a complex switch operator or sequence of if statements. and repeated in several places
Relatively rare use of switch and case operators is one of the hallmarks of object-oriented code. Often code for a single switch can be scattered in different places in the program. When a new condition is added, you have to find all the switch code and modify it.
As a rule of thumb, when you see switch you should think of polymorphism
Temporary Field
https://sourcemaking.com/refactoring/smells/temporary-field
Temporary fields get their values (and thus are needed by objects) only under certain circumstances. Outside of these circumstances, they are empty.
Oftentimes, temporary fields are created for use in an algorithm that requires a large amount of inputs. So instead of creating a large number of parameters in the method, the programmer decides to create fields for this data in the class. These fields are used only in the algorithm and go unused the rest of the time.
    public TimeSpan CalculateEstimate(
        IReadOnlyCollection<TimeSpan> durations)
    {
        if (durations == null)
            throw new ArgumentNullException(nameof(durations));
 
        if (durations.Count == 0)
            return this.defaultEstimate;
 
        this.durations = durations;
        this.CalculateAverage();
        this.CalculateStandardDeviation();
 
        var margin = TimeSpan.FromTicks(this.standardDeviation.Ticks * 3);
        return this.average + margin;
    }
 
    private void CalculateAverage()
    {
        this.average =
            TimeSpan.FromTicks(
                (long)this.durations.Average(ts => ts.Ticks));
    }
 
    private void CalculateStandardDeviation()
    {
        var variance =
            this.durations.Average(ts => 
                Math.Pow(
                    (ts - this.average).Ticks,
                    2));
        this.standardDeviation = 
            TimeSpan.FromTicks((long)Math.Sqrt(variance));
    }
This compiles! It also produces a result if you invoke the CalculateEstimate method. No exception is thrown, but the result returned is incorrect!
Not only is this code difficult to understand, it's also brittle. Furthermore, it's not thread-safe.
In Refactoring, the suggested cure is to extract a class that contains only the temporary fields. This refactoring is called Extract Class. Your first step can be to introduce a private, nested class within the containing class. In the case of the example, you might call this class DurationStatistics. It can be used from CalculateEstimate in this way:
public TimeSpan CalculateEstimate(
    IReadOnlyCollection<TimeSpan> durations)
{
    if (durations == null)
        throw new ArgumentNullException(nameof(durations));
 
    if (durations.Count == 0)
        return this.defaultEstimate;
 
    var stats = new DurationStatistics(durations);
    var margin = TimeSpan.FromTicks(stats.StandardDeviation.Ticks * 3);
    return stats.Average + margin;
}
It's now much clearer what's going on. You have some statistics based on the durations, and those contain both the average and the standard deviation. The flow of data is also clear: you need the stats to get the average and the standard deviation, and you need stats.StandardDeviation in order to calculate the margin, and the marginbefore you can calculate the return value. If you attempt to move those lines of code around, it's no longer going to compile.
This solution is also thread-safe.
As you can see, the Estimator class now only has a single read-only field, which ensures that it isn't temporary.
The original intent of not passing parameters around is still preserved, so this solution still adheres to that Clean Code principle.
The DurationStatistics class lazily calculates the average and standard deviation, and memoizes their results (or rather, that's what Lazy<T> does).
Instances of DurationStatistics have a shorter lifetime than the containing Estimator object.

Refused Bequest
https://sourcemaking.com/refactoring/smells/refused-bequest
If a subclass uses only some of the methods and properties inherited from its parents, the hierarchy is off-kilter. The unneeded methods may simply go unused or be redefined and give off exceptions.
Someone was motivated to create inheritance between classes only by the desire to reuse the code in a superclass. But the superclass and subclass are completely different.
  • If inheritance makes no sense and the subclass really does have nothing in common with the superclass, eliminate inheritance in favor of Replace Inheritance with Delegation.
  • If inheritance is appropriate, get rid of unneeded fields and methods in the subclass. Extract all fields and methods needed by the subclass from the parent class, put them in a new subclass, and set both classes to inherit from it (Extract Superclass).
Alternative Classes with Different Interfaces
https://sourcemaking.com/refactoring/smells/alternative-classes-with-different-interfaces
Two classes perform identical functions but have different method names.
The programmer who created one of the classes probably didn't know that a functionally equivalent class already existed.
Try to put the interface of classes in terms of a common denominator:
  • Rename Methods to make them identical in all alternative classes.
  • Move Method, Add Parameter and Parameterize Method to make the signature and implementation of methods the same.
  • If only part of the functionality of the classes is duplicated, try using Extract Superclass. In this case, the existing classes will become subclasses.
  • After you have determined which treatment method to use and implemented it, you may be able to delete one of the classes.
Change Preventers
These smells mean that if you need to change something in one place in your code, you have to make many changes in other places too. Program development becomes much more complicated and expensive as a result.

Divergent Change
Divergent Change is when many changes are made to a single class. Shotgun Surgeryrefers to when a single change is made to multiple classes simultaneously.
You find yourself having to change many unrelated methods when you make changes to a class. For example, when adding a new product type you have to change the methods for finding, displaying, and ordering products.
Divergent change occurs when one class is commonly changed in different ways for different reasons.
This is just another way of identifying classes that violate the Single Responsibility Principle. If you find yourself changing the same class to implement totally different things, then that class has too many responsibilities.
class UserPresenter < SimpleDelegator
  def full_name
    "#{first_name} #{last_name}"
  end
end

class ActionAuthorizer
  def authorized?(action, user)
    user.is_admin?
  end
end

class User
  # nothing here?
end
Now when you remember that full names can include salutations or middle initials, you have one class to change. And when you implement a better way of authorizing actions, you have one class to change.

This is, I think, a reflection of our tendency to base objects on real world things. Users are people, and people have full names. People are authorized to do things, and so on. Real world things are a complex web of responsibilities, behaviors and attributes. Why try to model that rat's nest into a single class? Keep it simple, unlike the real world.
Shotgun Surgery
https://sourcemaking.com/refactoring/smells/shotgun-surgery
Making any modifications requires that you make many small changes to many different classes.
  • Use Move Method and Move Field to move existing class behaviors into a single class. If there is no class appropriate for this, create a new one.
  • If moving code to the same class leaves the original classes almost empty, try to get rid of these now-redundant classes via Inline Class.
http://javaonfly.blogspot.in/2016/09/code-smell-and-shotgun-surgery.html
Cause of Shotgun surgery smell:
1.    Due to poor separation of concern.
2.    Fail to understand the responsibility. Often due to misunderstanding (Single responsibility principle)
3.    Not identifying the common behavior or behavior with a slight change.
4.    Fail to introduce proper design pattern.
Consequences of Shotgun Surgery:
1.    Lots of duplicates code
2.    Taking more time to develop a small feature.
3.    Unmaintainable code base.

Parallel Inheritance Hierarchies

Whenever you create a subclass for a class, you find yourself needing to create a subclass for another class.
Parallel Inheritance Hierarchies code smells occurs when an inheritance tree depends on another inheritance tree by composition and they maintain a special relation one subclass of dependent inheritance must dependent one a particular subclass of another Inheritance


Dispensables
A dispensable is something pointless and unneeded whose absence would make the code cleaner, more efficient and easier to understand.

Comments
Duplicate Code
Lazy Class
Understanding and maintaining classes always costs time and money. So if a class doesn't do enough to earn your attention, it should be deleted.
Data Class
A data class refers to a class that contains only fields and crude methods for accessing them (getters and setters). These are simply containers for data used by other classes. These classes do not contain any additional functionality and cannot independently operate on the data that they own.
  • Review the client code that is used by the class. In it, you may find functionality that would be better located in the data class itself. If this is the case, use Move Method and Extract Method to migrate this functionality to the data class.
Dead Code
Speculative Generality
Sometimes code is created "just in case" to support anticipated future features that never get implemented. As a result, code becomes hard to understand and support.
I’m just reviewing a project’s code. I found a common pattern used in their code base. Every class implements an Interface. Each interface is only implemented by one class. Even more interesting, this interface is not exposed outside. In other words, its not exposed as part of the API.
Then my question is
Why do we need the interface? Why can’t we just use the class directly?


Couplers
All the smells in this group contribute to excessive coupling between classes or show what happens if coupling is replaced by excessive delegation.

Feature Envy
A method accesses the data of another object more than its own data.
This smell may occur after fields are moved to a data class. If this is the case, you may want to move the operations on data to this class as well.
As a basic rule, if things change at the same time, you should keep them in the same place. Usually data and functions that use this data are changed together (although exceptions are possible).
  • If a method uses functions from several other classes, first determine which class contains most of the data used. Then place the method in this class along with the other data. Alternatively, use Extract Method to split the method into several parts that can be placed in different places in different classes.
https://elearning.industriallogic.com/gh/submit?Action=PageAction&album=recognizingSmells&path=recognizingSmells/featureEnvy/featureEnvyExample&devLanguage=Java
https://help.semmle.com/wiki/display/JAVA/Feature+envy
Description: A method that uses more methods or variables from another (unrelated) class than from its own class violates the principle of putting data and behavior in the same place.
Feature envy refers to situations where a method is "in the wrong place", because it does not use many methods or variables of its own class, but uses a whole range of methods or variables from some other class. This violates the principle of putting data and behavior in the same place, and exposes internals of the other class to the method.
If it is inappropriate to move the entire method, see if all the dependencies on the other class are concentrated in just one part of the method. If so, they can be moved into a method of their own. This method can then be moved to the other class and called from the original method.
If a class is envious of functionality defined in a superclass, perhaps the superclass needs to be re-written to become more extensible and allow its subtypes to define new behavior without them depending so deeply on the superclass's implementation. The template method pattern may be useful in achieving this.
Inappropriate Intimacy
https://sourcemaking.com/refactoring/smells/inappropriate-intimacy
One class uses the internal fields and methods of another class.
Keep a close eye on classes that spend too much time together. Good classes should know as little about each other as possible. Such classes are easier to maintain and reuse.
https://sourcemaking.com/refactoring/change-bidirectional-association-to-unidirectional

Problem

You have a bidirectional association between classes, but one of the classes does not use the other's features.

Solution

Remove the unused association.
A bidirectional association is generally harder to maintain than a unidirectional one, requiring additional code for properly creating and deleting the relevant objects. This makes the program more complicated.
In addition, an improperly implemented bidirectional association can cause problems for garbage collection (in turn leading to memory bloat by unused objects).


Message Chains
Middle Man
If a class performs only one action, delegating work to another class, why does it exist at all?
Do not delete middle man that have been created for a reason:
  • A middle man may have been added to avoid interclass dependencies.
  • Some design patterns create middle man on purpose (such as Proxy and Decorator).
Incomplete Library Class
https://sourcemaking.com/refactoring/smells/incomplete-library-class
Sooner or later, libraries stop meeting user needs. The only solution to the problem – changing the library – is often impossible since the library is read-only.
The author of the library has not provided the features you need or has refused to implement them.

Problem

A utility class does not contain the method that you need and you cannot add the method to the class.

Solution

Add the method to a client class and pass an object of the utility class to it as an argument.
https://sourcemaking.com/refactoring/hide-delegate
Problem
The client gets object B from a field or method of object А. Then the client calls a method of object B.

Solution
Create a new method in class A that delegates the call to object B. Now the client does not know about, or depend on, class B.

Benefits
Hides delegation from the client. The less that the client code needs to know about the details of relationships between objects, the easier it is to make changes to your program.
Drawbacks
If you need to create an excessive number of delegating methods, server-class risks becoming an unneeded go-between, leading to an excess of Middle Man.


Replace Delegation with Inheritance
Problem
A class contains many simple methods that delegate to all methods of another class.

Solution
Make the class a delegate inheritor, which makes the delegating methods unnecessary.



https://sourcemaking.com/refactoring/introduce-local-extension
Create a new class containing the methods and make it either the child or wrapper of the utility class.
https://8thlight.com/blog/georgina-mcfadyen/2017/01/19/common-code-smells.html

Long Methods

Refuse Bequest

If a class inherits from a base class but doesn't use any of the inherited fields or methods, developers should ask themselves if inheritance really is the right model. Signs of this code smell may be that the inherited methods go unused, or are overridden with empty method bodies
Inheritance should be used when a class wants to reuse the code in its superclass. If the classes diverge and the subclass no longer needs that functionality, the hierarchy should be broken and delegation considered instead.
Alternatively, to keep some inheritance, remove the unused fields and methods from the subclass and create a new layer that the objects can inherit from. For example:
posts/2017-01-09-common-code-smells/animal-hierarchy.png

Data Clumps

Where multiple method calls take the same set of parameters, it may be a sign that those parameters are related. To keep the group of parameters together, it can be useful to combine them together in a class. This can help aid organisation of code.

Duplicate Code

Middle Man

When a class exists just to delegate to another, a developer should ask themselves what its real purpose is. Sometimes this is the result of a refactoring task, where logic has been moved out of a class gradually, leaving an almost empty shell.
For every class that exists, there is an overhead of maintenance. Make sure the class justifies its existence, and if it doesn't, go ahead and remove it.

Primitive Obsession

Primitive types give little in terms of domain context. A String id field could ultimately contain any sort of value. Where primitives have a domain meaning, wrap them in a small class to represent the idea. Often the class is expanded to include methods to add to the class.
For example:
Rather than
String id = product.getId();
if (isValid(id)) {
  ...
}
use
ProductId id = product.getId();
if (id.isValid()) {
  ...
}

class ProductId {

  public bool isValid() {
    ...
  }
}

Comments

Can comments be trusted? Where comments are re-iterating what can be read by a developer, they may provide little value, especially when they have not been updated and no longer reflect the intent of the current code.
Rather than adding a comment to clarify a piece of code, think about whether the code can be refactored such that the comment is no longer needed.
It may be possible to provide a more descriptive name that provides the same clarity as the comment, meaning the comment can disappear, resulting in more intuitive and readable code.
https://blog.jetbrains.com/idea/tag/code-smells/
https://blog.jetbrains.com/idea/2017/08/code-smells-iteration/

https://blog.jetbrains.com/idea/2017/08/code-smells-iteration/
The Smell: Iteration
Example 1: Set instead of List
    boolean hasName(String storedName) {

        return getLoadNames().stream()

                             .anyMatch(storedName::equals);

    }
The getLoadNames method is not a simple getter, as you might expect from the name (but misleading naming is not the topic of this blog post, nor is caching the results of an operation that only needs to be done once, so let’s gloss over those smells for now).  It processes some data in order to calculate the list of names. The implementation details aren’t important for the purpose of this article, we’re just going to do the simplest thing that works and change the method to return a Set.
Example 2: Map instead of List

    public MappedField getMappedFieldByJavaField(final String name) {

        for (final MappedField mf : persistenceFields) {

            if (mf.getJavaFieldName().equals(name)) {

                return mf;

            }
        }
        return null;
    }

https://blog.jetbrains.com/idea/2017/08/code-smells-deeply-nested-code/
Solution 1: Java 8 to the rescue
    public MappedField getMappedField(final String storedName) {
        for (final MappedField mf : persistenceFields) {
            for (final String n : mf.getLoadNames()) {
                if (storedName.equals(n)) {
                    return mf;
                }
            }
        }
        return null;
    }
Optional<String> found = persistenceFields.stream()
                                          .flatMap(mappedField -> mappedField.getLoadNames().stream())
                                          .filter(storedName::equals)
                                          .findFirst();
Solution 2: Better Encapsulation
What’s the real problem here?  It seems like the original code is reaching deep inside another object, stealing all its data, riffling through it and then only caring about the top level object.  Removing the loop syntax, what you have is effectively:
This what the Law of Demeter warns us against, therefore suggests to me we consider a tell-don’t-ask approach.  Wouldn’t it be prettier to ask mappedField if one of its names matched the name we wanted?
    public boolean hasName(String storedName) {
        return getLoadNames().stream()
                             .anyMatch(storedName::equals);

    }
    public MappedField getMappedField(final String storedName) {
        return persistenceFields.stream()
                                .filter(mf -> mf.hasName(storedName))
                                .findFirst()
                                .orElse(null);
    }
    public Optional<MappedField> getMappedField(final String storedName) {
        return persistenceFields.stream()
                                .filter(mf -> mf.hasName(storedName))
                                .findFirst();
    }
  • Deeply nested code, usually loops and/or conditionals
  • Chained method calls that are all getters (or other non-builder, non-DSL method chains).
Possible solutions

  • Consider tell don’t ask. Where the code previously grabbed internal data and did checks against it, move those checks into the object containing the data and create a method with a useful name to help other objects answer their questions.
  • Examine your encapsulation. Does the data really need to leak out into other classes? Should data and/or responsibilities be relocated so that operations on the data are moved closer to the data itself?
  • Collection Encapsulation. Sometimes this pattern is a sign that you’re using a data structure as a domain object.  Consider encapsulating this data structure (Collection, array, etc) inside a domain object of its own and adding helper methods to let other objects ask it questions rather than poke around its internals.
https://blog.jetbrains.com/idea/2017/09/code-smells-if-statements/
Step 1: Place Guard Conditions at the Start
Some of these if statements are controlling the flow – returning from the method if possible.  What I would like to do is move these as early in the method as possible, so my brain can spot these special conditions and then ignore them for the rest of the method.
Why does that matter?  We could add a comment to this if statement to clarify its meaning, but my preference is usually to wrap these conditions in a method that returns a boolean, and give it a useful name
    private static boolean isOperator(String propertyPath) {
        return propertyPath.startsWith("$");
    }

Actually if I preferred, I could better reflect this in the code by pushing the “not” into the method and calling it isFieldPath, if I think that’s more understandable

Step 2: Remove Logic for Controlling the Iteration
The abuse of the for loop syntax in this code pains me:
for (int i = 0; ; ) {
Now we’re using a proper for loop with a guard condition, we don’t need the break any more (especially as it’s right at the end of the method).  The for loop itself will make sure don’t go past the end of the array.  So we can remove this else statement completely.
Step 4: Replace Multiple Checks of Value

We see fieldIsArrayOperator is checked twice, and both times we care about if it is not an array operator.  My question, therefore, is what do we do, if anything, when it is an array operator?  Here we actually need to either go into depth digging through the code, or apply some domain knowledge.

Step 6: Collapse two ifs into an if else


  1. We don’t have to clutter our navigation/validation checks with conditionals for managing the looping behaviour, we let a standard for loop take care of that
  2. We use more else statements, so it’s easier to reason around which code branches are executed and when
  3. We’ve moved early-exit code as close to the top of the method as possible, so that we can see those cases and ignore them when we’re looking further down the code
  4. We’ve extracted tiny methods to encapsulate our conditions (once we’d simplified them) so that we understand better what each if statement is really checking, and hopefully why.
Symptoms:
  • Lots of if statements
  • Very few else branches
  • The same / similar conditions appearing in multiple branches (isPresent, fieldIsArrayOperator)
  • Flow control being managed manually
Possible Solutions:

  • Flipping the logic.  Humans find it easier to reason about the positive (rather than negative) values, so try to use positive terms where possible. If a negation is necessary, you might wrap this in a method that’s easier to understand.
  • If a value is being checked regularly (like fieldIsArrayOperator was), it may be easier to reason about it if you pull out just this case and deal with that individually. Then you can remove the check from all the other places.
  • Consider whether an else is going to be more readable than a series of ifs.  Remember that a series of ifs (without else) can potentially lead to all branches being executed, whereas an explicit else shows that this is an either/or.  This is easier for a human to reason about, but it’s also potentially more efficient for the computer too. You may need to consider flipping the logic in order to make use of else statements.
  • Once you’ve done as much as you can for reducing duplicate use of values in the conditions, consider refactoring each condition, even possibly single checks, into its own method with a meaningful name.  This can enormously improve the readability of if statements (see: Extract Method, Decompose Conditional)
  • Cases which are guard cases, basic validation or simple checks for something that might be an exit case could be moved to the top of any method (if this doesn’t impact the method’s overall logic) so that the reader of the code doesn’t have to reason about these cases throughout the method (see: Replace Nested Conditional with Guard Clauses).
  • On the other hand, fall-through exceptional cases may belong at the end of the method or in a final else, so we understand this is a “when all else fails” situation.
https://blog.jetbrains.com/idea/2017/08/code-smells-mutation/


https://blog.jetbrains.com/idea/2017/08/code-smells-null/
Given that lots of these *Converter classes seem to return a null value in the decode method, it seems reasonable that we might want to change the Converter superclass (an abstract class called TypeConverter) to return Optional here.  But a closer look at the code shows what’s actually happening: we see exactly the same pattern happening again and again – the method checks if a value passed in was null, and if so returns null:
The first question is can fromDBObject actually be null? The code is sufficiently complicated that it’s difficult to tell, but theoretically it seems plausible, since this is a value from the database.
A quick search shows that all instances of this method are actually only called from one central place, so instead of making this check in 21 different places, we can just do it in a single place and from there on assume fromDBObject can no longer be null.
Solution: @NotNull parameter

Symptoms:

  • Null checks extensively used throughout the application code, without it being clear to you, the developer, what null really signifies or if the value can really be null at all.
  • Explicit return null

Possible Solutions:

  • Optional.  Not a solution to everywhere you find a null.  But if a method is deliberately returning a null value to mean “not found” or “I don’t have one of those”, this is a good candidate for returning an Optional.
This is a great example of where null means two different things. The first null means we were given a null input, therefore we give you a null output.  Seems fairly valid, if a little unhelpful. The second null means “an error happened and I couldn’t give you a value, so here, have null”. Presumably, a null ID is a valid response, given the first case, but it would probably be more helpful to the caller to know what the error was and deal with it accordingly.  Even if the Exception is such that the caller cannot deal with it, it’s pretty unfriendly to catch the Exception and return null, especially when the Exception isn’t even logged.  This is a really good way to hide genuine problems.  Exceptions should be handled where they’re found, or passed on in some useful way, they should not be swallowed unless you really really know it’s not an error.
So this null means “something unexpected happened and I don’t know what to do about it, so I’m going to give you null and hope that’s OK”, which is not at all clear to the caller.  These cases should definitely be avoided, and Optional is not going to be your solution. The solution is to implement much clearer error handing.

The encode method on the same TypeConverter class can also return null.  But unlike decode, there are valid times when the converter implementations explicitly return null:


1. Anonymous Inner Types

Anywhere you encounter an inner class is a good place to consider using a lambda expression. 


2. Comparators
New helper methods on Comparator, combined with method references, can make it clear which property is being used for sorting:
list.sort(Comparator.comparingInt(String::length));

With these helper methods, you’ll get the results in ascending order unless you specify otherwise:
list.sort(Comparator.comparingInt(String::length).reversed());

3. Classes With No State
Often you come across classes with names ending in Util or Helper that contain static methods but no state of their own. Now that interfaces support static methods, these classes may be better as interfaces so no one can accidentally sneak state into a type that is only meant to contain functions. Comparator is a perfect example of when static methods on interfaces can be useful and powerful.Similarly, you may come across abstract classes with no state, only methods with behavior and abstract methods
that are designed to be overridden. If there’s no state,

these can be converted to interfaces. Why? If you’re fortunate enough to only have a single abstract method
in your class, you can turn it into a FunctionalInterface and implement this type with a lambda expression.


4. Nested for/if Statements
The Streams API was designed to give us much greater flexibility when querying collections.

return fields.stream() .filter(this::meetsCriteria) .collect(Collectors.toList());
Sometimes, for loops with an inner if statement may be refactored to anyMatch or findFirst:

5. Multiple Operations on a Collection
// collect messages for logging
List<LogLine> lines = new ArrayList<>();
for (Message message : messages) {
lines.add(new LogLine(message));
}
// sort
Collections.sort(lines);
// log them
for (LogLine line : lines) {
line.log(LOG);
}

messages.stream()
.map(LogLine::new)
.sorted()
.forEach(logLine -> logLine.log(LOG));

6. Using an Iterator to Remove Elements - ArrayList
strings.removeIf (current -> current.endsWith(“jnilib”));

7. Null Checks
public static Optional<String> findString
(String wanted) {
List<String> strings = new ArrayList<>();
return strings.stream()
.filter(current -> current.
equals(wanted))
.findFirst();
}


In Java 8, you can do:
items.removeIf(i -> predicate(i));
Where you used to do:
for (Iterator it = items.iterator(); it.hasNext();) {
  if (predicate(it.next())) {
    it.remove(); 
  } 
}

If the collection in question is a linked list of size n and m elements are removed, the full iteration with removals is O(n + m) = O(n) since iteration steps and removal from the list are constant time operations and m is limited by n. If, however, the data structure of the collection is one where removal is relatively expensive compared to iteration, the complexity becomes less favorable. For an ArrayList a single removal step is O(n) since all items at higher indices have to be moved. There are still n iteration steps to be made so the full sequence of removals becomes O(m*n) which is O(n^2) assuming worst case with the number of removals being a constant fraction of the array size.
In more pragmatic terms, what happens in the ArrayList case is that for each element that is removed, all elements of higher indices are moved to fill the gap in the backing array. This is clearly suboptimal since that operation is repeated for each removal. Thus, to reach its final destination an element will visit all locations between the starting position and the destination. In case many elements are removed, this may be a considerable amount of work in vain.



Posted by Jeffery at 9:52 PM

Labels

Review (572) System Design (334) System Design - Review (198) Java (189) Coding (75) Interview-System Design (65) Interview (63) Book Notes (59) Coding - Review (59) to-do (45) Linux (43) Knowledge (39) Interview-Java (35) Knowledge - Review (32) Database (31) Design Patterns (31) Big Data (29) Product Architecture (28) MultiThread (27) Soft Skills (27) Concurrency (26) Cracking Code Interview (26) Miscs (25) Distributed (24) OOD Design (24) Google (23) Career (22) Interview - Review (21) Java - Code (21) Operating System (21) Interview Q&A (20) System Design - Practice (20) Tips (19) Algorithm (17) Company - Facebook (17) Security (17) How to Ace Interview (16) Brain Teaser (14) Linux - Shell (14) Redis (14) Testing (14) Tools (14) Code Quality (13) Search (13) Spark (13) Spring (13) Company - LinkedIn (12) How to (12) Interview-Database (12) Interview-Operating System (12) Solr (12) Architecture Principles (11) Resource (10) Amazon (9) Cache (9) Git (9) Interview - MultiThread (9) Scalability (9) Trouble Shooting (9) Web Dev (9) Architecture Model (8) Better Programmer (8) Cassandra (8) Company - Uber (8) Java67 (8) Math (8) OO Design principles (8) SOLID (8) Design (7) Interview Corner (7) JVM (7) Java Basics (7) Kafka (7) Mac (7) Machine Learning (7) NoSQL (7) C++ (6) Chrome (6) File System (6) Highscalability (6) How to Better (6) Network (6) Restful (6) CareerCup (5) Code Review (5) Hash (5) How to Interview (5) JDK Source Code (5) JavaScript (5) Leetcode (5) Must Known (5) Python (5)

Popular Posts