HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Elevating my code for an elevator program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
programelevatorforelevatingcode

Problem

I've been working on this elevator program for quite some time now and finally finished it and made it work and I'm pretty proud of myself, but I'd like to see ways how to refactor my code so I can learn for the future. By refactoring I mean, making the code look better, maybe by using less lines of codes.

```
public class Test {

public static void main( String[] args) {

Random r = new Random();
List nums = new ArrayList();
for ( int i = 0; i positiveNumbers = removeNegativeNumbers( nums);
Set negativeNumbers = removePositiveNumbers( nums);

System.out.println( positiveNumbers.size() + " + " + negativeNumbers.size() + " should be 5000");

// part 2
List positiveList = repeatElements( positiveNumbers, 5);
List negativeList = repeatElements( negativeNumbers, 5);
Collection merged = mergeAndSort( negativeList, positiveList);
System.out.println( merged.size() + " sorted elements");
// part 3
Map nums2 = new ConcurrentHashMap();

for ( int i = 0; i repeatElements( Collection nums, int repeat) {
List result = new ArrayList();
for ( int i = 0; i nums2) {

int currentSize = 0;
while ( currentSize != nums2.size()) {
currentSize = nums2.size();
int evenKey = -1;

for ( int key : nums2.keySet()) {
if ( key % 2 == 1) {
continue;
}
if ( evenKey mergeAndSort( List nums1, List nums2) {
List result = new ArrayList();
result.addAll( nums1);
result.addAll( nums2);
Collections.sort( result);
return result;
}

private static Set removePositiveNumbers( List list) {
for ( int i = 0; i 0) {
list.remove( i);
}
}
return new HashSet( list);
}

private static Set removeNegativeNumbers( List list) {
for ( int i = 0; i ( list);
}

pr

Solution

Your code is an odd collection of loosely related functions, many of which have solutions in the Java Streams API. In addition, you have odd signatures that don't make sense.

I look at methods like:

private static Set removeNegativeNumbers( List list) {
    for ( int i = 0; i ( list);
}


And I think: Why:

  • return a Set when the input is a List



  • why remove the data from the input list, and then return it in a set (you do know that remove() on ArrayList is really slow?)



  • have an input of a list of Integer, why not an array of int? If you have Integer, will you have Long too?, etc.



  • if you return a set, why force a List as input, why not just a Collection, or Iterable?



For multiple reasons, mostly for performance, though, I would have something like:

private static  Set removeNegativeNumbers( Iterable data) {
    return StreamSupport.stream(data.spliterator())
            .filter(n -> Math.signum(n.doubleValue()) >= 0.0)
            .collect(Collectors.toSet());
}


That will return all non-negative values for any of the number types.

It iwll work for List, List, List, Set, etc.

Using generics, streams, and appropriate collections is the first step in making reusable code.

Code Snippets

private static Set<Integer> removeNegativeNumbers( List<Integer> list) {
    for ( int i = 0; i < list.size(); i++) {
        if ( list.get( i) < 0) {
            list.remove( i);
        }
    }
    return new HashSet<Integer>( list);
}
private static <N extends Number> Set<N> removeNegativeNumbers( Iterable<N> data) {
    return StreamSupport.stream(data.spliterator())
            .filter(n -> Math.signum(n.doubleValue()) >= 0.0)
            .collect(Collectors.toSet());
}

Context

StackExchange Code Review Q#104432, answer score: 3

Revisions (0)

No revisions yet.