[ https://issues.apache.org/jira/browse/MNG-7899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17777831#comment-17777831 ]
ASF GitHub Bot commented on MNG-7899: ------------------------------------- sebastien-doyon commented on code in PR #1296: URL: https://github.com/apache/maven/pull/1296#discussion_r1367135089 ########## maven-embedder/src/main/java/org/apache/maven/cli/transfer/FileSizeFormat.java: ########## @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.cli.transfer; + +import java.text.DecimalFormat; +import java.text.DecimalFormatSymbols; +import java.util.Locale; + +/** + * Formats file size with the associated <a href="https://en.wikipedia.org/wiki/Metric_prefix">SI</a> prefix + * (GB, MB, kB) and using the patterns <code>#0.0</code> for numbers between 1 and 10 + * and <code>###0</code> for numbers between 10 and 1000+ by default. + * + * @see <a href="https://en.wikipedia.org/wiki/Metric_prefix">https://en.wikipedia.org/wiki/Metric_prefix</a> + * @see <a href="https://en.wikipedia.org/wiki/Binary_prefix">https://en.wikipedia.org/wiki/Binary_prefix</a> + * @see <a + * href="https://en.wikipedia.org/wiki/Octet_%28computing%29">https://en.wikipedia.org/wiki/Octet_(computing)</a> + */ +public class FileSizeFormat { + enum ScaleUnit { + BYTE { + @Override + public long bytes() { + return 1L; + } + + @Override + public String symbol() { + return "B"; + } + }, + KILOBYTE { + @Override + public long bytes() { + return 1000L; + } + + @Override + public String symbol() { + return "kB"; + } + }, + MEGABYTE { + @Override + public long bytes() { + return KILOBYTE.bytes() * KILOBYTE.bytes(); + } + + @Override + public String symbol() { + return "MB"; + } + }, + GIGABYTE { + @Override + public long bytes() { + return MEGABYTE.bytes() * KILOBYTE.bytes(); + } + ; + + @Override + public String symbol() { + return "GB"; + } + }; + + public abstract long bytes(); + + public abstract String symbol(); + + public static ScaleUnit getScaleUnit(long size) { + if (size < 0L) { + throw new IllegalArgumentException("file size cannot be negative: " + size); + } + + if (size >= GIGABYTE.bytes()) { + return GIGABYTE; + } else if (size >= MEGABYTE.bytes()) { + return MEGABYTE; + } else if (size >= KILOBYTE.bytes()) { + return KILOBYTE; + } else { + return BYTE; + } + } + } + + private DecimalFormat smallFormat; + private DecimalFormat largeFormat; + + public FileSizeFormat(Locale locale) { + smallFormat = new DecimalFormat("#0.0", new DecimalFormatSymbols(locale)); + largeFormat = new DecimalFormat("###0", new DecimalFormatSymbols(locale)); + } + + public String format(long size) { + return format(size, ScaleUnit.getScaleUnit(size)); + } + + public String getScaleSymbol(long size) { + return ScaleUnit.getScaleUnit(size).symbol(); + } + + @SuppressWarnings("checkstyle:magicnumber") + private String format(long size, ScaleUnit unit) { + if (size < 0L) { + throw new IllegalArgumentException("file size cannot be negative: " + size); + } + + double scaledSize = (double) size / unit.bytes(); + + if (unit == ScaleUnit.BYTE) { + return largeFormat.format(size); Review Comment: Would have been nice, but the NumberFormat.format() method takes only StringBuffer as parameter, the synchronized StringBuilder. They are not interchangeable and I am not sure we should switch StringBuilder for StringBuffer for that.... > Various memory usage improvements > --------------------------------- > > Key: MNG-7899 > URL: https://issues.apache.org/jira/browse/MNG-7899 > Project: Maven > Issue Type: Improvement > Components: Design, Patterns & Best Practices, Embedding, > General, Logging > Affects Versions: 4.0.0-alpha-2 > Reporter: sebastien > Priority: Major > > Some optimisations can be applied to the code to reduce the use of temporary > objects. > Typical improvements identified are: > * reduce scope of temporary objects creation to avoid creating when not > needed. Example : > {code:java} > public String toString() { > StringBuilder sb = new StringBuilder(512); > if (isEmpty()) { > return "empty"; > } > for (MetadataGraphVertex v : vertices) { > .....{code} > can be replaced by > {code:java} > public String toString() { > if (isEmpty()) { > return "empty"; > } > StringBuilder sb = new StringBuilder(512); > for (MetadataGraphVertex v : vertices) { > .....{code} > * Reuse StringBuilder objects in loops by setting its length to zero > * Use the StringBuilder.append() with index to avoid String.substring(). > Example: > > {code:java} > int idx = resourceName.lastIndexOf('/'); > buffer.append(idx < 0 ? resourceName : resourceName.substring(idx + 1)); > {code} > can be replaced by > > {code:java} > int idx = resourceName.lastIndexOf('/'); > if (idx < 0) { > buffer.append(resourceName); > } else { > buffer.append(resourceName, idx + 1, resourceName.length()); > } {code} > > > * Replace dynamic string creation static constants when possible > * Avoid creating temporary strings with + operator when the final > destination can be used instead, like PrintStream.print() method > * Avoir using StringBuilder.append() method and util method MessageUtils.a() > when the final destination can be used instead, like PrintStream.print() > method > -- This message was sent by Atlassian Jira (v8.20.10#820010)