patternjavaMinor
Elevating my code for an elevator program
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
```
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:
And I think: Why:
For multiple reasons, mostly for performance, though, I would have something like:
That will return all non-negative values for any of the number types.
It iwll work for
Using generics, streams, and appropriate collections is the first step in making reusable code.
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.