Jag har några förslag till dig.

Extrahera en del av logiken till metoder.

I din kod, när frågan är insert och get har du två stora kodblock som liknar; du kan extrahera till en metod och återanvända metoden i båda sektionerna.

Jag föreslår en metod som returnerar en boolean baserat på if -villkoret så att du blir kunna ställa in variablerna currValue och currKey.

  long hashMap(String[] queryType, int[][] query) { //[...] switch (currQuery) { //[...] case "insert": if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } values.put(query[i][0], query[i][1]); break; //[...] } //[...] } private boolean didWeCopiedValuesToMap(Integer currKey, Integer currValue, Map<Integer, Integer> values, HashMap<Integer, Integer> copiedValues) { if (currKey != 0 || currValue != 0) { Set<Integer> keys = values.keySet(); for (Integer key : keys) { copiedValues.put(key + currKey, values.get(key) + currValue); } values.clear(); values.putAll(copiedValues); return true; } return false; }  

För att kontrollera den aktuella frågan currQuery kan du extrahera var och en av dem med en metod.

 private boolean isGet(String currQuery) { return "get".equals(currQuery); } private boolean isAddToKey(String currQuery) { return "addToKey".equals(currQuery); } private boolean isAddToValue(String currQuery) { return "addToValue".equals(currQuery); } private boolean isInsert(String currQuery) { return "insert".equals(currQuery); }  

Använd alltid primitiverna när det är möjligt

När du vet att det är omöjligt att få ett nollvärde med numret, försök att använda primitiverna. De tar mindre minne och är snabbare än omslagsklassen.

Före

 Integer currKey = 0; Integer currValue = 0;  

Efter

 int currKey = 0; int currValue = 0;  

Försök sätta mindre kod i switch block

Enligt min mening blir koden mindre läsbar när det finns mer än 3 rader koder i ett växelblock; Jag föreslår att du konverterar den till en is-else-if. Den här konverteringen gör koden kortare och mer läsbar.

Före

 switch (currQuery) { case "insert": if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } values.put(query[i][0], query[i][1]); break; case "addToValue": currValue += values.isEmpty() ? 0 : query[i][0]; break; case "addToKey": currKey += values.isEmpty() ? 0 : query[i][0]; break; case "get": if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } sum += values.get(query[i][0]); }  

Efter

 if ("insert".equals(currQuery)) { if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } values.put(query[i][0], query[i][1]); } else if ("addToValue".equals(currQuery)) { currValue += values.isEmpty() ? 0 : query[i][0]; } else if ("addToKey".equals(currQuery)) { currKey += values.isEmpty() ? 0 : query[i][0]; } else if ("get".equals(currQuery)) { if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } sum += values.get(query[i][0]); }  

Refactored code

  long hashMap(String[] queryType, int[][] query) { long sum = 0; int currKey = 0; int currValue = 0; Map<Integer, Integer> values = new HashMap<>(); for (int i = 0; i < queryType.length; i++) { String currQuery = queryType[i]; if (isInsert(currQuery)) { if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } values.put(query[i][0], query[i][1]); } else if (isAddToValue(currQuery)) { currValue += values.isEmpty() ? 0 : query[i][0]; } else if (isAddToKey(currQuery)) { currKey += values.isEmpty() ? 0 : query[i][0]; } else if (isGet(currQuery)) { if (didWeCopiedValuesToMap(currKey, currValue, values)) { currValue = 0; currKey = 0; } sum += values.get(query[i][0]); } } return sum; } private boolean isGet(String currQuery) { return "get".equals(currQuery); } private boolean isAddToKey(String currQuery) { return "addToKey".equals(currQuery); } private boolean isAddToValue(String currQuery) { return "addToValue".equals(currQuery); } private boolean isInsert(String currQuery) { return "insert".equals(currQuery); } private boolean didWeCopiedValuesToMap(int currKey, int currValue, Map<Integer, Integer> values) { HashMap<Integer, Integer> copiedValues = new HashMap<>(); if (currKey != 0 || currValue != 0) { Set<Integer> keys = values.keySet(); for (Integer key : keys) { copiedValues.put(key + currKey, values.get(key) + currValue); } values.clear(); values.putAll(copiedValues); return true; } return false; }  

Det mest dyr operation är addToKey x som lägger till x till alla nycklar på kartan, eftersom du i huvudsak måste skapa en ny inmatningsnyckel, värde + x i din hashmap och ta bort den gamla inmatningsnyckeln, värdet. För att undvika behovet av att cacha den gamla posten medan den itererar över kartan kan du skilja mellan två fall:

x > 0, om du har iterera över en keyset beställt fallande, det finns inget behov av att cacha de gamla posterna

x < 0, samma tillvägagångssätt men keyset ordnas stigande

Eftersom du använder hashmap finns ingen nyckelordning garanterad, så du behöver en data struktur för att lagra nycklar som ska beställas, innan det går igenom nycklar som nedan:

private static void addtoKey(Map<Integer, Integer> map, int i) { if (i != 0) { List<Integer> list = new ArrayList<>(map.keySet()); if (i > 0) { Collections.sort(list, Collections.reverseOrder()); } else { Collections.sort(list); } for(int key : list) { map.put(key + i, map.get(key)); map.remove(key); } } } 

Jag utesluter fallet 0 eftersom map förblir orörd. Andra operationer behöver inte ordning på nycklarna och som redan föreslagits kan det vara bättre att försöka isolera alla operationer i en privat metod.

Kommentarer

• Tack @dariosicily för svaret. Är inte ’ t sortering varje gång medan addToKey är också kostsamt ?. Eller kan jag använda a SortedMap för att hålla infogningsordningen fallande. Gilla, SortedMap<Integer, Integer>values = new TreeMap<Integer, Integer>(Collections.reverseOrder());
• @Praveen Du är välkommen. Ja det är sortering varje gång, men med ArrayList efter sortering fortsätter du linjärt.Jag dömdes till att du bara kunde använda HashMap; om du kan använda TreeMap istället för HashMap kan du använda en iterator och en omvänd iterator och itera över din TreeMap på ett rakt sätt.

Ändrad version av Johnbots svar utan en extra klass. Jag tror att den extra klassen är överdriven och snarare distraherar från algoritmen, eftersom jag måste söka igenom mycket kod (en hel del panna) för att se vad som händer. Det är inte den extra klassen som gör bearbetningsslingan mycket enklare. Det är algoritmen.

Ytterligare ändringar:

• keyOffset är inte klart för mig i vilken riktning det förskjuts, så jag döpte om det till addedToKey (även för värde).
• Beställde operationen namn som i problemspecifikationen, både för att hålla mig nära specifikationen och för att den ordningen är mer vettig för mig.
• Introducerade args för att spara lite kodupprepning.
• Används long / Long för allt, inte bara för summan. När allt kommer omkring kan tillägg till nycklarna / värdena få dem att rinna över om vi bara använder int / Integer.

static long hashMap(String[] queryType, int[][] query) { Map<Long, Long> map = new HashMap<>(); long sum = 0, addedToKey = 0, addedToValue = 0; for (int i = 0; i < query.length; i++) { int[] args = query[i]; switch (queryType[i]) { case "insert": map.put(args[0] - addedToKey, args[1] - addedToValue); break; case "get": sum += map.get(args[0] - addedToKey) + addedToValue; break; case "addToKey": addedToKey += args[0]; break; case "addToValue": addedToValue += args[0]; } } return sum; } 

Kommentarer

• Varför lägger du enhetligt till addedToKey till värde ’ s-tangenten fungerar inte, men subtraherar den för insert och get -åtgärderna arbete?

Vad sägs om att bara lagra ett offsetvärde för nycklar och värden och bygga omslagsmetoder runt hashmaps get / put-metoder för att ta hänsyn till denna förskjutning.