Java.Util.HashMap · Pull Request #257 · tiny-express/native

ghost · 2017-09-18T13:28:00Z

Edited + checked old source.
Built + Add test for new method.

codecov · 2017-09-18T14:00:21Z

Codecov Report

Merging #257 into development will decrease coverage by 1.26%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##           development     #257      +/-   ##
===============================================
- Coverage        95.62%   94.35%   -1.27%     
===============================================
  Files              186      173      -13     
  Lines            16565    15443    -1122     
  Branches           270      267       -3     
===============================================
- Hits             15840    14571    -1269     
- Misses             725      872     +147

Impacted Files	Coverage Δ
java/util/HashMap/HashMap.hpp	`99.2% <ø> (ø)`	⬆️
kernel/Test.hpp	`52.71% <ø> (-1.31%)`	⬇️
java/util/HashMap/HashMapTest.cpp	`100% <100%> (ø)`	⬆️
java/util/Date/Date.cpp	`0% <0%> (-100%)`	⬇️
java/lang/String/String.hpp	`96.85% <0%> (-0.05%)`	⬇️
java/util/ArrayList/ArrayListTest.cpp	`100% <0%> (ø)`	⬆️
java/lang/Object/Object.hpp	`96.87% <0%> (ø)`	⬆️
java/lang/String/StringTest.cpp	`100% <0%> (ø)`	⬆️
java/security/MessageDigest/MessageDigestTest.cpp
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21a3ecb...5109168. Read the comment docs.

TuanAnhNguyen69

Please check for some mistakes in comments

TuanAnhNguyen69 · 2017-09-23T19:01:38Z

java/util/HashMap/HashMapTest.cpp

-	result = anotherMap.get(notExpectedKey);
+	hashMap.put("Key1", "Value of Key1");
+	hashMap.put("Key2", "Value of Key2");
+	HashMap<String, String> anotherMap = (HashMap<String, String>) hashMap.clone();


Please remove this cast, is't not necessary

Thanks. Fixed.

TuanAnhNguyen69 · 2017-09-23T19:03:04Z

java/util/HashMap/HashMapTest.cpp


-TEST (JavaUtil, HashMapReplaceSpecifiedValue) {
-	// Given valid hash map to test
+TEST(JavaUtil, HashMapReplace2) {


Please rename this test

Thanks. Fixed.

TuanAnhNguyen69 · 2017-09-23T19:03:47Z

kernel/type.h

 #define FALSE 0
 #define NOT_FOUND -1
 #define MAX_STRING_LENGTH 100000
+#define BiConsumer std::function


I think we should comment this and wait for BiConsumer class

Please remove this definition because it's wrong. Should be BiConsumer implementation

Thanks. Fixed.

TuanAnhNguyen69 · 2017-09-23T19:04:36Z

java/util/HashMap/HashMap.hpp

+			 *
+			 * @param action
+			 */
+			void forEach (BiConsumer<void(Key, Value)> action) {


I think we should comment this and wait for BiConsumer class

Thanks. Fixed.

TuanAnhNguyen69 · 2017-09-23T19:08:20Z

java/util/HashMap/HashMap.hpp

+			 *
+			 * @return a set view of the keys contained in this map
+			 */
+//			Set<Key> keySet() {


Set class is available, what do you really need in Set class?

loint

Please remove all binaries in your PR.

loint · 2017-09-26T18:51:25Z

kernel/type.h

 #define FALSE 0
 #define NOT_FOUND -1
 #define MAX_STRING_LENGTH 100000
+#define BiConsumer std::function


Please remove this definition because it's wrong. Should be BiConsumer implementation

dthongvl

Please remove unused files and ignore them.
Please update methods' s comment that use null

dthongvl · 2017-10-16T10:28:26Z

kernel/Type.hpp


 #define NOT_FOUND -1
 #define MAX_STRING_LENGTH 100000
+#define BiConsumer std::function


Please remove this

Thanks. Fixed.

dthongvl · 2017-10-16T10:55:15Z

java/util/HashMap/HashMap.hpp

+             * Compares the specified object
+             * with this map for equality.
+             *
+             * @param object


Please update param

Thanks. Fixed.

dthongvl · 2017-10-16T10:58:35Z

java/util/HashMap/HashMap.hpp

+             *
+             * @param action
+             */
+            void forEach(BiConsumer<void(Key, Value)> action) {


Please help me disable this function until BiConsumer is implemented

Thanks. Fixed.

dthongvl · 2017-10-16T10:58:56Z

java/util/HashMap/HashMap.hpp

+             * {Map.Entry hashCode()} on each element (entry) in the
+             * set, and adding up the results.
+             *
+             * @return 	the hash code value for this map


Thanks. Fixed.

dthongvl · 2017-10-16T10:59:05Z

java/util/HashMap/HashMap.hpp

+             * Object.
+             *
+             * This implementation iterates over entrySet(), calling
+             * {Map.Entry hashCode()} on each element (entry) in the


Please remove { }

Thanks. Fixed.

dthongvl · 2017-10-16T11:01:00Z

java/util/HashMap/HashMap.hpp

+             *  Set of hash map entries
+             *
+             * Returns a Set view of the mappings contained in this map.
+             * @return Set<Map.Entry<Key,Value>> - a set view of the mappings contained in this map


Please help me update return data type.

Thanks. Fixed.

dthongvl · 2017-10-16T11:01:43Z

java/util/HashMap/HashMap.hpp

+
+            /**
+             * Returns the value to which the specified key is mapped,
+             * or null if this map contains no mapping for the key


default constructor or something, not null

Thanks. Fixed.

dthongvl · 2017-10-16T11:05:37Z

java/util/HashMap/HashMapTest.cpp

-	assertEquals(100, counter);
+    HashMap<String, String> hashMap;
+
+    for (int index = 1; index <= 100; index++) {


Please create int index; outside the loop

Thanks. Fixed.

tiny-express · 2017-10-20T17:09:04Z

This PR will be close because no progress.

ghost requested review from TuanAnhNguyen69, dthongvl and loint September 18, 2017 13:28

ghost self-assigned this Sep 18, 2017

dthongvl changed the title ~~Java.util.Hash map~~ Java.util.HashMap Sep 18, 2017

ghost added reviewing and removed pull request labels Sep 19, 2017

loint changed the title ~~Java.util.HashMap~~ Java.Util.HashMap Sep 19, 2017

ghost added the fixed label Sep 22, 2017

TuanAnhNguyen69 suggested changes Sep 23, 2017

View reviewed changes

TuanAnhNguyen69 self-assigned this Sep 23, 2017

loint requested changes Sep 26, 2017

View reviewed changes

ghost added in progress fixed and removed fixed in progress labels Oct 2, 2017

ghost added in progress fixed and removed fixed in progress labels Oct 10, 2017

thuydunq added changes requested and removed fixed labels Oct 15, 2017

ghost added fixed and removed changes requested labels Oct 16, 2017

dthongvl suggested changes Oct 16, 2017

View reviewed changes

dthongvl added changes requested and removed fixed labels Oct 16, 2017

#50 Fix PR

55f6093

ghost added the fixed label Oct 16, 2017

ghost removed the changes requested label Oct 16, 2017

#271 Fix assertEquals(String, String)

ec575fb

dthongvl added changes requested and removed fixed labels Oct 16, 2017

Update README.md

5109168

tiny-express merged commit 4e74c7e into development Oct 20, 2017

tiny-express deleted the java.util.HashMap branch October 20, 2017 19:33

Conversation

ghost commented Sep 18, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TuanAnhNguyen69 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loint left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dthongvl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiny-express commented Oct 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

ghost commented Sep 18, 2017 •

edited by ghost

Loading

codecov bot commented Sep 18, 2017 •

edited

Loading