Write into the same CSV file multiple times I implemented an OutputWriterService.class to write content in the same file multiple times.

The writeToCsvFile method is called round about 50.000 times and includes a list of descrptions which also needs to iterate and write to the file.

Is it a good way to instantiate a new Filewriter, BufferedWriter and PrintWriter for every call?

public void writeToCsvFile(List<CustomTypeDescription> descriptions){
        try(FileWriter fw = new FileWriter(FILE_NAME, true);
            BufferedWriter bw = new BufferedWriter(fw);
            PrintWriter out = new PrintWriter(bw))
        {
            for(CustomTypeDescription description : descriptions){
                out.println(description.toString());
            }

            //here the fw.flush()?

        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public void createOutputFile() throws IOException {
        try (Writer writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(FILE_NAME), "utf-8"))) {
        }
    }

Update:

Is it necessary to flush File-, Buffered- and Printwriter one by one or just the FileWriter. I would do it after the loop?

If you want to avoid the creation of the *Writers on each call of writeToCsvFile() method, one of solutions would be as follows.

Change OutputWriterService class to implement AutoCloseable interface. This will allow to put its instances into a try-with-resources block. The Writers created inside the original method will become constants in the changed class and they will be closed inside the close() method when the routine exits from the block (normally or with an exception):

public class OutputWriterService implements AutoCloseable {

  private static final String FILE_NAME = "FILE";

  private final FileWriter fw;
  private final BufferedWriter bw;
  private final PrintWriter out;

  public OutputWriterService() throws IOException {
    this.fw = new FileWriter(FILE_NAME, true);
    this.bw = new BufferedWriter(fw);
    this.out = new PrintWriter(bw);
  }

  @Override
  public void close() throws IOException {
    this.fw.close();
    this.bw.close();
    this.out.close();
  }

  public void writeToCsvFile(List<CustomTypeDescription> descriptions){
    // ...
  }

}

writeToCsvFile() method becomes shorter:

public void writeToCsvFile(List<CustomTypeDescription> descriptions){
  try {
    for(CustomTypeDescription description : descriptions){
      out.println(description.toString());
    }
  } catch (IOException e) {
    e.printStackTrace();
  }
}

By the way, the code of the original createOutputFile() method looks very suspicious. A cleaner way would be to use

Files.createFile(Paths.get(FILE_NAME));

The instance of the BufferedWriter can also be created differently:

Files.newBufferedWriter(Paths.get(FILE_NAME),
                        StandardCharsets.UTF_8,
                        StandardOpenOption.APPEND);

This makes the FileWriter instance unnecessary.

Now, the user of your OutputWriterService class should proceed as follows:

try (OutputWriterService service = new OutputWriterService()) {

  // May be called as many times as desired, depending on the algorithm
  // The same instances of the writers will be reused.
  service.writeToCsvFile(descriptions);

} // + process the exception or rethrow it


Graceful JavaScript fallback for external web service failure I use the src.sencha.io service to resize images on the fly according to device width, saving lots of bandwidth on mobiles etc.

Sometimes the service fails randomly with a 503 error, so a fallback is needed. I've written this JS to fallback to "non-sencha" urls, and emit an error event, if the first image requested through sencha fails.

This is my first open-sourced bit of code, so am keen to get feedback on my:

code correctness
whether this prototype-based design pattern is appropriate
alternative better patterns
comment/code clarity

Even just confirming whether it's good would be helpful, as I don't have any formal programming training to be confident.

Github repo here.

jsfiddle with test case prepared for you here.

/** SENCHA SRC IMAGE FALLBACK ***************************************
 *
 * Author: Josh Harrison (http://www.joshharrison.net/)
 * URL: https://github.com/ultrapasty/sencha-src-image-fallback
 * Version: 1.0.0
 * Requires: jQuery
 *
 * Tests to see if the src.sencha.io service is down, and if so,
 * falls back to local images, by removing the sencha domain
 * from the src.
 *
 * Can be instantiated with `new SenchaSRCFallback().init();`
 * Or `var instance = new SenchaSRCFallback().init();`
 *
 * Emits the event `senchafailure` if sencha fails.
 * Listen with `instance.onsenchafailure = func;`
 *
 */

;(function(window, $) {

  var SenchaSRCFallback = function() {

    this.sencha_path_identifier = '.sencha.io/';
    this.sencha_path_regex = /http:\/\/src[1-4]?\.sencha\.io\//i;
    this.$imgs = null;

  };
  SenchaSRCFallback.prototype = {

    init : function() {
      this.$imgs = $("img[src*='" + this.sencha_path_identifier + "']");

      if(this.$imgs.length) {
        this.test_sencha_availability();
      }

      return this;
    },

    test_sencha_availability : function() {
      var t = this, img = new Image();

      img.onerror = function() {
        $(t).trigger("senchafailure");
        t.fallback_to_local_srcs();
      };

      img.src = this.$imgs[0].getAttribute("src");
    },

    fallback_to_local_srcs : function() {
      var t = this;

      this.$imgs.each(function() {
        this.setAttribute("src", this.getAttribute("src").replace(t.sencha_path_regex, ""));
      });
    }

  };

  window.SenchaSRCFallback = SenchaSRCFallback;

})(window, jQuery);

// Example usage:

// Instantiate the fallback
var senchafallback = new SenchaSRCFallback().init();

// Listen for failure like this:
senchafallback.onsenchafailure = function() {
    console.log("It failed.");
    // log failure event in google analytics, etc
};

Images would be initially routed through src.sencha.io to avoid downloading twice. With the above JS, should sencha fail, these image tags:

... would become the following:

I like the idea, from a once over:

Since you use an IIFE I would declare sencha_path_identifier and sencha_path_regex outside of the constructor, then you can access them without specifying this.
I would do either function SenchaSRCFallback() { or var SenchaSRCFallback = function SenchaSRCFallback() {, anonymous functions are a pain to troubleshoot
Why do you require the caller to call init()? In my mind this call should be done within the constructor.

This is something I might borrow at some point.


Optimizing string handling function How can I optimize the following function? It checks whether two strings have a certain number of similar consecutive characters. As it is now, it is very slow, so how can it be improved in terms of speed/performance:

#include <stdio.h>

int overlap(char *str, const char *substr, int length);

int main(void)
{
    if (overlap("abcef01239", "abckef01798", 4))
    {
        puts("Match ...");
    }
    return 0;
}

int overlap(char *str, const char *substr, int length)
{
    const char *_sub = substr;
    while (*str)
    {
        int n = 0;
        int a = *str;
        int b = *substr;
        while (a && b)
        {
            if (a != b)
                break;

            n++;
            if (n == length)
                return 1;

            a = str[n];
            b = substr[n];
        }
        if (!*substr)
        {
            str++;
            substr = _sub;
        }
        else
        {
            substr++;
        }
    }
    return 0;
}

Const correctness

We don't modify the contents of str, so pass it as a const char*.  This allows us to give string literals as arguments without compiler warnings.

Check the arguments

What if str or substr is null?  What does it mean for length to be negative?  We should use size_t for indexing and lengths.

int overlap(const char *str, const char *substr, size_t length)
{
    if (!str || !substr)
        /* invalid arguments */
        return 0;

Add more tests

I'm not 100% sure from the description what the code is supposed to do.  That can be made clearer by providing more tests.  I recommend using a test framework to help identify the failures, but we can make our own for this:

int test(const char *a, const char *b, size_t len,
         int expected, const char *file, int line, const char *message)
{
    const int actual = overlap(a, b, len);
    if (actual == expected) return 0;

    fprintf(stderr, "%s:%d: %s should be %d, but got %d\n",
            file, line,
            message, expected, actual);
    return 1;
}

#define TEST(a, b, len, expected) \
    test(a, b, len, expected, __FILE__, __LINE__, "overlap(" #a ", " #b  ", " #len ")")

int main(void)
{
    return
        + TEST(NULL, NULL, 1, 0)
        + TEST("abc", "def", 0, 1)
        + TEST("abc", "abc", 4, 0)
        + TEST("abc", "abc", 3, 1)
        + TEST("abc", "bcd", 3, 0)
        + TEST("abc", "bcd", 2, 1)
        + TEST("bcd", "abc", 2, 1)
        + TEST("abcef01239", "abckef01798", 4, 1);
}

Algorithm

We're going more times over the data than we need to - although it looks like we're scaling as O(n²), it's actually O(n³), because of the if/else at the end of the loop that resets substr from time to time.

What we need to do is for each offset of str and substr, walk along the pair of them, counting character matches as we go (reset the count on a non-match).  If the count reaches length, then we've found a substring match.

As a worked example, let's test "ab1cdefg" against "bcde2":

ab1cdefg
bcde2
.....

In that first pass, nothing matched.  Now shift one of the strings by one character (i.e. index by n+1 instead of by n):

ab1cdefg
 bcde2
 1....

The longest run was 1, when we matched the b.  Next iteration:

ab1cdefg
  bcde2
  .123.

Here, we matched c, d and e successively, for a run length of 3.

In code, that looks something like:

#include <string.h>

/* internal method - returns true if a and b are equal for a run of
   length, without shifting */
static int overlap_run(const char *a, const char *b, size_t length)
{
    size_t count = 0;
    while (*a && *b) {
        /* add one to count, or reset it to 0 */
        if (*a == *b)
            ++count;
        else
            count = 0;

        if (count == length)
            /* match found */
            return 1;

        ++a, ++b;
    }
    return 0;
}

int overlap(const char *a, const char *b, size_t length)
{
    if (!a || !b)
        /* invalid arguments */
        return 0;

    if (!length)
        /* every pair of strings has a zero-length match */
        return 1;

    const size_t la = strlen(a);
    const size_t lb = strlen(b);

    if (length > la || length > lb)
        /* strings are too short to match */
        return 0;

    for (size_t offset = 0;  offset <= la - length;  ++offset)
        if (overlap_run(a+offset, b, length))
            return 1;

    for (size_t offset = 1;  offset <= lb - length;  ++offset)
        if (overlap_run(a, b+offset, length))
            return 1;

    /* no match */
    return 0;
}

You might be able to avoid pipeline stalls by replacing

    if (*a == *b)
        ++count;
    else
        count = 0;

With

    count += (*a == *b);
    count *= (*a == *b);

But you'll need to benchmark to find out on your hardware.


First Python program: pick and display alien characteristics This is my first Python program and all it does is print out a description of the specific "Alien Class" that the user inputs.

Looking for tips and pointers and what I can do better and what I am doing well.

# Initializing the alien army with different alien classes

alien_army = {
    'rogue': {
        'name': 'rogue',
        'color': 'green',
        'size': 'small',
        'speed': 'fast',
        'damage': 'small'
    },
    'warrior': {
        'name': 'warrior',
        'color': 'blue',
        'size': 'medium',
        'speed': 'average',
        'damage': 'heavy'
    },
    'hunter': {
        'name': 'hunter',
        'color': 'dark green',
        'size': 'average',
        'speed': 'average',
        'damage': 'average'
    }
}

# Adding an Alien class 'Monk' to the alien_army

alien_army['monk'] = {
    'name': 'monk',
    'color': 'light gray',
    'size': 'small',
    'speed': 'quick',
    'damage': 'average'
}

class_names = []
for k, v in alien_army.items():
    class_names.append(k)

# Function that shows the alien that the user typed

def showPickedAlien(a):
    print(f"{a['name'].upper()}'S are {a['color'].title()} in color, {a['size'].lower()} in size, {a['speed'].lower()} in speed and they do a {a['damage'].lower()} amount in damage.")

# While loop flag to run or stop loop

input_enabled = True

# While loop to keep displaying the alien class that the user inputs

while input_enabled:
    user_input = input("Which class do you want to see? ").lower()
    input_alien = alien_army.get(user_input)

    # Checking if user entered a valid alien class name

    if user_input in class_names:
        showPickedAlien(input_alien)
    else:
        print(f"{user_input.upper()} is not a valid alien class name.")

    # Asking user if they want to see another alien class or not

    input_for_quit = input("Do you want to see another class? ").lower()

    # Determining if the user wants the loop to stop or not

    if input_for_quit == 'no' or input_for_quit == 'n':
          print("Goodbye.")
          input_enabled = False
    elif input_for_quit == 'yes' or input_for_quit == 'y':
          input_enabled = True

Good job on your first Python program!

You used a Dictionary to store all the alien classes. There are many other alternatives to do the same task. The most common one for this kind of problem is Python Classes (https://www.w3schools.com/python/python_classes.asp).

You could simply create a class called myArmy and then create your soldiers as objects.

#Create a class called myArmy
class myArmy:

  #Use the __init__() function to assign values for name, color...
  def __init__(self, name, color, size, speed, damage):

    self.name = name
    self.color = color
    self.size = size
    self.speed = speed
    self.damage = damage

#Create your soldiers (Referred to as "Objects" here. The __init__() function is called automatically every time the class is being used to create a new object.
rogue = myArmy('rogue', 'green', 'small', 'fast', 'small')
warrior = myArmy('warrior', 'green', 'medium', 'average', 'heavy')
hunter = myArmy('hunter', 'dark green', 'average', 'average', 'average')

#Access Rogue information for example
print(rogue.name, rogue.color, rogue.size, rogue.speed, rogue.damage)

Next step should be to give them attack/defense values and attack speed, and let the user make them battle each other!


Making the font as big as possible without overflowing each div I made a function that makes font sizes in
s as big as possible without overflowing. It takes a long time to run. How do I optimize this?

$(document).ready(() => {
  $('.text-fit').each((index, element) => {
    let currentElement = $(element);
    let fontSize = 999;
    currentElement.wrapInner("

");
    while (currentElement.find('.wrapper').innerHeight() > currentElement.innerHeight()) {
      fontSize -= 1;
      currentElement.css('font-size', fontSize + 'px');
    }
  });
});

No particular comment on the style, but FWIW, I had to implement the same thing recently — also in jQuery — and this is how I did it. Assume we have an outer <div> named time_display_div that we don't want to overflow, and nested immediately inside that we have a <span> named time_display_span that we're trying to make as big as possible.

function effective_fraction(span, div) {
    var x = span.width() / div.width();
    var y = span.height() / div.height();
    return Math.max(x, y);
}

function update_display(msg) {
    // Change the text of the time display,
    // and massage the font size until it fills the screen.
    var div = $('#time_display_div');
    var span = $('#time_display_span');
    var font_size = parseInt(span.css('font-size'));
    span.text(msg);
    while (effective_fraction(span, div) < 0.90) {
        font_size += 10;
        span.css('font-size', font_size + 'px');
    }
    while (effective_fraction(span, div) > 0.90) {
        font_size -= 2;
        span.css('font-size', font_size + 'px');
    }
}

Basically the only difference is that I'm skipping through the sizes at ten times the speed you are; and I'm using .height() instead of .innerHeight(), but I can confirm that at least in Safari it makes no difference which one I use.

So at least if anyone comes up with a better solution here, it'll be useful to more people than just yourself. :)


Error-Handling Class and Logging I am relatively new to C# and would like to see if what I have been using for my exception handling, message and logging is along industry standards. I have created a simple class for the Error Handler and for the logging I use Log4Net. After reading Clean Code, I was thinking about separating DisplayMessage into 2 different procedures for the message and logging. The log file is usually written to a file server so multiple users can access it; I have enabled MinimalLock for this purpose. For my example, I have changed the log path to C:\Temp\. An example of a project I use the Error Handler class and logging in is on GitHub. For reference, I am using Visual Studio 2017 Community.

ErrorHandler.cs

using System;
using System.Windows.Forms;
using log4net;

[assembly: log4net.Config.XmlConfigurator(Watch = true)]

/// <summary>
/// Used to handle exceptions
/// </summary>
public class ErrorHandler
{
    private static readonly ILog log = LogManager.GetLogger(typeof(ErrorHandler));

    /// <summary>
    /// Used to produce an error message and create a log record
    /// <example>
    /// <code lang="C#">
    /// ErrorHandler.DisplayMessage(ex);
    /// </code>
    /// </example>
    /// </summary>
    /// <param name="ex">Represents errors that occur during application execution.</param>
    /// <remarks></remarks>
    public static void DisplayMessage(Exception ex)
    {
        System.Diagnostics.StackFrame sf = new System.Diagnostics.StackFrame(1);
        System.Reflection.MethodBase caller = sf.GetMethod();
        string currentProcedure = (caller.Name).Trim();
        string errorMessageDescription = ex.ToString();
        errorMessageDescription = System.Text.RegularExpressions.Regex.Replace(errorMessageDescription, @"\r\n+", " "); //the carriage returns were messing up my log file
        string msg = "Contact your system administrator. A record has been created in the log file." + Environment.NewLine;
        msg += "Procedure: " + currentProcedure + Environment.NewLine;
        msg += "Description: " + ex.ToString() + Environment.NewLine;
        log.Error("[PROCEDURE]=|" + currentProcedure + "|[USER NAME]=|" + Environment.UserName + "|[MACHINE NAME]=|" + Environment.MachineName + "|[DESCRIPTION]=|" + errorMessageDescription);
        MessageBox.Show(msg, "Unexpected Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }

}

Usage Example:

/// <summary>
/// Return the count of items in a delimited list
/// </summary>
/// <param name="valueList">Represents the list of values in a string </param>
/// <param name="delimiter">Represents the list delimiter </param>
/// <returns>the number of values in a delimited string</returns>
public int GetListItemCount(string valueList, string delimiter)
{
    try
    {
        string[] comboList = valueList.Split((delimiter).ToCharArray());
        return comboList.GetUpperBound(0) + 1;

    }
    catch (Exception ex)
    {
        ErrorHandler.DisplayMessage(ex);
        return 0;
    }
}

Log4Net

app.config

<log4net>
    <appender name="ConsoleAppender" type="log4net.Appender.ConsoleAppender">
        <layout type="log4net.Layout.PatternLayout">
            <conversionPattern value="%date [%thread] %-5level %logger [%ndc] - %message%newline"/>
        </layout>
    </appender>
    <appender name="FileAppender" type="log4net.Appender.FileAppender">
        <file value="C:\Temp\MyLogFile.log"/>
        <appendToFile value="true"/>
        <lockingModel type="log4net.Appender.FileAppender+MinimalLock"/>
        <layout type="log4net.Layout.PatternLayout">
            <conversionPattern value="%date|%-5level|%message%newline"/>
        </layout>
    </appender>
    <root>
        <level value="ALL"/>
        <appender-ref ref="FileAppender"/>
    </root>
</log4net>

I'm not familiar with log4net, so I will focus on the code itself.

Here is a list of what could be improved:

--- Style ---
ErrorHandler should be a static class to prevent newing up an instance of it
Lack of using namespace: Generally, you should declare all of them, unless:

there is an conflic in which case you can alias it: using A = Some.Namespace.A
a namespace is particularly noisy and is polluting the intellisense

Lack of var: Using the type name or the full name can make the code difficult to scan through. It is more difficult to locate a variable when they are not aligned. For the most part, the right hand side assignment should give enough of hint.

System.Diagnostics.StackFrame sf = new System.Diagnostics.StackFrame(1);
System.Reflection.MethodBase caller = sf.GetMethod();
string currentProcedure = (caller.Name).Trim();
// --- for comparison
var sf = new System.Diagnostics.StackFrame(1);
var caller = sf.GetMethod();
var currentProcedure = (caller.Name).Trim();

Lack of "spacing": Nobody likes reading a wall of text, and a wall of code is no better. You can divide the logical blocks of your code with some empty lines: (Think of them as paragraphs)

// gather context
var sf = new System.Diagnostics.StackFrame(1);
var caller = sf.GetMethod();

// format message
var currentProcedure = (caller.Name).Trim();
var errorMessageDescription = ex.ToString();
errorMessageDescription = System.Text.RegularExpressions.Regex.Replace(errorMessageDescription, @"\r\n+", " "); //the carriage returns were messing up my log file
var msg = "Contact your system administrator. A record has been created in the log file." + Environment.NewLine;
msg += "Procedure: " + currentProcedure + Environment.NewLine;
msg += "Description: " + ex.ToString() + Environment.NewLine;

// handle exception
log.Error("[PROCEDURE]=|" + currentProcedure + "|[USER NAME]=|" + Environment.UserName + "|[MACHINE NAME]=|" + Environment.MachineName + "|[DESCRIPTION]=|" + errorMessageDescription);
MessageBox.Show(msg, "Unexpected Error", MessageBoxButtons.OK, MessageBoxIcon.Error);

Use of unnecessary parenthesis that doesn't highlight order of operation:

string currentProcedure = (caller.Name).Trim();
string[] comboList = valueList.Split((delimiter).ToCharArray());

--- code ---

string[] comboList = valueList.Split((delimiter).ToCharArray());
return comboList.GetUpperBound(0) + 1;

You can simply return comboList.Length here.

 /// <param name="delimiter">Represents the list delimiter </param>
 string[] comboList = valueList.Split((delimiter).ToCharArray());

You are using it as multiple delimiters, and not just one. The name should reflect that.

 /// <returns>the number of values in a delimited string</returns>

The return of value 0 in error handling should be documented in the <returns> or <remarks> tag.

public static class ErrorHandler
{
    public static void DisplayMessage(Exception ex)
    {
        var sf = new System.Diagnostics.StackFrame(1);
        var caller = sf.GetMethod();
        var currentProcedure = caller.Name.Trim();

        var logMessage = string.Concat(new Dictionary<string, string>
        {
            ["PROCEDURE"] = currentProcedure,
            ["USER NAME"] = Environment.UserName,
            ["MACHINE NAME"] = Environment.MachineName,
            ["DESCRIPTION"] = ex.ToString().Replace("\r\n", " "), // the carriage returns were messing up my log file
        }.Select(x => $"[{x.Key}]=|{x.Value}|"));
        log.Error(logMessage);

        // pick one:
        var userMessage = new StringBuilder()
            .AppendLine("Contact your system administrator. A record has been created in the log file.")
            .AppendLine("Procedure: " + currentProcedure)
            .AppendLine("Description: " + ex.ToString())
            .ToString();
        var userMessage = string.Join(Environment.NewLine,
            "Contact your system administrator. A record has been created in the log file.",
            "Procedure: " + currentProcedure,
            "Description: " + ex.ToString(),
        ) + Environment.NewLine;
        MessageBox.Show(userMessage, "Unexpected Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
}


Working with list in Java This code has a method that I call for in main.

I am given a list with temperatures.  What I need to do is to write out the biggest absolute difference of 2 neighbour numbers.  For example, if I have {1, 3, 4, 10, 7, 4}, the result is 6 (between 4 and 10) and also all other pairs that have the same difference.

I have this code written down but I feel it looks kind of clumsy. I'd be very grateful for any suggestions and improvements.

public class DN4 {
    static void najvecjaZaporednaRazlika(double stevila[]){
        double maxRaz = 0;
        double[] tStevil = new double[0];
        for (int i = 0; i < stevila.length - 1; i++){
            double a = stevila[i];
            double b = stevila[i+1];
            double absRaz = a - b;
            if (absRaz < 0){
                absRaz = -1 * absRaz;
            }
            if (absRaz == maxRaz){
                double tStevilStara[] = tStevil;
                tStevil = new double[tStevilStara.length+2];
                tStevil[tStevil.length - 2] = a;
                tStevil[tStevil.length - 1] = b;
                for (int t = 0; t < tStevilStara.length; t++){
                    tStevil[t] = tStevilStara[t];
                }
            }
            else if (absRaz > maxRaz){
                maxRaz = absRaz;
                tStevil = new double[2];
                tStevil[0] = a;
                tStevil[1] = b;
            }
        }
        System.out.printf("Najvecja razlika: %.2f\n",maxRaz);
        System.out.printf("Pari,ki dosezejo maksimalno razliko: ");
        for (int i = 0; i < tStevil.length - 1; i = i+2){
            if (i == 0){
                System.out.printf("(%.2f, %.2f)",tStevil[i],tStevil[i+1]);
            }
            else{
                System.out.printf(", (%.2f, %.2f)",tStevil[i],tStevil[i+1]);
            }
        }
        System.out.println();
    }

    public static void main (String args[]){
        double kredarica2011[] = {-7.8, -6.5, -5.7, -1.3, 1.7,  5,
                               5.6,  9.2,  7.1,  0.5, 0.3, -6.3};
        najvecjaZaporednaRazlika(kredarica2011);
    }
}

General

Your code style is neat. I like that you are using System.out.printf instead of concatenating strings. You should consider using %n instead of \nin the format because it is platform independent.
Math.abs() - Math.abs() is a core function that you should use instead of doing it yourself.
Growing the tStevil array - you do this manually, and you increase and decrease the size often. This can be inefficient. It may be better to keep a big-enough version, and have a separate int tStevillength variable to keep track of how much is actually populated.
This code here:

    for (int i = 0; i < tStevil.length - 1; i = i+2){
        if (i == 0){
            System.out.printf("(%.2f, %.2f)",tStevil[i],tStevil[i+1]);
        }
        else{
            System.out.printf(", (%.2f, %.2f)",tStevil[i],tStevil[i+1]);
        }
    }

would be better done with less code repeated:

for (int i = 0; i < tStevil.length - 1; i = i+2){
    if (i > 0){
        System.out.print(", ");
    }
    System.out.printf("(%.2f, %.2f)",tStevil[i],tStevil[i+1]);
}

Algorithm.

Your algorithm is close to what I would have done. The only difference is that I would keep an array of indexes of the first member of a temperature pair....

Also, sometimes it is easier to loop from i = 1, instead of looping to i < length - 1

My code would look something like:

int pairlen = 0;
int[] pairs = new int[stevila.length];
double maxdiff = Double.MIN_VALUE;
for (int i = 1; i < stevila.length; i++) {
    double diff = Math.abs(stevila[i] - stevila[i - 1]);
    if (diff > maxdiff) {
        maxdiff = diff;
        pairlen = 0;
    }
    if (diff == maxdiff) {
        pairs[pairlen++] = [i - 1];
    }
}

This way, at the end of the loop, you have the index to the first pair of pairlen indexes for values with the largest diff.

You can then loop through that list using for (i = 0; i < pairlen; i++) { .... } and pull the actual values back from the stevila array.

This is not actually very different fdrom the way you have done it, but it is a bit neater.

Nice job.


Routing Java objects using conditional consumers As per a previous question here and answers I got, I used it to get a new implementation. I found that I have to make two implementations match once "to ignore other consumers after matching once" and match all "to execute all consumers its filter matching the input", and made a common interface to abstract usage of two implementations:

Common interface

public interface Router<T> {
Router<T> chain(Predicate<T> p, Consumer<T> c);
Consumer<T> toConsumer();
}

Match All:

public interface RouterMatchAll<T> extends Consumer<T>, Router<T> {
static <T> RouterMatchAll<T> of(Predicate<T> p1, Consumer<T> c1) {
    BiFunction<Predicate<T>, Consumer<T>, RouterMatchAll<T>> preFun = (p, c) -> i -> Optional
            .ofNullable(i).filter(p).ifPresent(c);
    return preFun.apply(p1, c1);
}

@Override
public default RouterMatchAll<T> chain(Predicate<T> p, Consumer<T> c) {
    return this.andThen(RouterMatchAll.of(p, c))::accept;
}

@Override
public default Consumer<T> toConsumer() {
    return this;
}

}

Match Once:

public interface RouterMatchOnce<T> extends Function<Optional<T>, Optional<T>>,
    Router<T> {

static <T> RouterMatchOnce<T> of(Predicate<T> p1, Consumer<T> c1) {
    BiFunction<Predicate<T>, Consumer<T>, RouterMatchOnce<T>> preFun = (p,
            c) -> o -> {
        if (o.filter(p).isPresent()) {
            c.accept(o.get());
            return Optional.empty();
        }
        return o;
    };
    return preFun.apply(p1, c1);

}

@Override
public default RouterMatchOnce<T> chain(Predicate<T> p, Consumer<T> c) {
    return this.andThen(RouterMatchOnce.of(p, c))::apply;
}

@Override
public default Consumer<T> toConsumer() {
    return i -> this.apply(Optional.ofNullable(i));
}

}

For MatchAll, I used Consumer to use its andThen, which consumes the same input. For MatchOnce, I had to use Function<Optional,Optional> and its andThen, use the output of first function to be used in next function, and I needed to change the data to Optional.empty() after consuming once to not consume it then.

Concerns I want your feedback about:

toConsumer() in RouterMatchAll is just return this and I don't like it but couldn't find another option for abstraction.
Names are not nice as it is not a router any more. It has to be something like filter, interceptor, or ConditionalConsumer. Please suggest a name for these interfaces, chain, and toConsumer methods.

Usage of the above:

Predicate<Integer> p1 = (Integer i) -> {
    System.out.println("call p1");
    return i < 5;
};
Predicate<Integer> p2 = (Integer i) -> {
    System.out.println("call p2");
    return i > 5;
};
Consumer<Integer> c1 = i -> System.out.println("less");
Consumer<Integer> c2 = i -> System.out.println("more");

Router<Integer> routerMatchAll = RouterMatchAll.of(p1, c1)
        .chain(p2, c2);
IntStream.range(0, 10).boxed().forEach(routerMatchAll.toConsumer());
System.out.println(">>>>>>>>>>>>>>>>>>>>>>");
Router<Integer> routerMatchOnce = RouterMatchOnce.of(p1, c1).chain(p2,
        c2);
IntStream.range(0, 10).boxed().forEach(routerMatchOnce.toConsumer());

Output:

  call p1
  less
  call p2
  call p1
  less
  call p2
  call p1
  less
  call p2
  call p1
  less
  call p2
  call p1
  less
  call p2
  call p1
  call p2
  call p1
  call p2
  more
  call p1
  call p2
  more
  call p1
  call p2
  more
  call p1
  call p2
  more
  >>>>>>>>>>>>>>>>>>>>>>
  call p1
  less
  call p1
  less
  call p1
  less
  call p1
  less
  call p1
  less
  call p1
  call p2
  call p1
  call p2
  more
  call p1
  call p2
  more
  call p1
  call p2
  more
  call p1
  call p2
  more

Again, I strongly recommend you stick to the methods and systems already provided in the Streams API. In this case, the ability to observe-without-changing the stream is given with the Stream.peek() method and the ability to conditionally remove a value from the stream is given with the Stream.filter() method. Using these two methods with a simple helper class, you can get the functionality you desire. Consider your existing "use case":

Predicate<Integer> p1 = (Integer i) -> {
    System.out.println("call p1");
    return i < 5;
};
Predicate<Integer> p2 = (Integer i) -> {
    System.out.println("call p2");
    return i > 5;
};
Consumer<Integer> c1 = i -> System.out.println("less");
Consumer<Integer> c2 = i -> System.out.println("more");

Router<Integer> routerMatchAll = RouterMatchAll.of(p1, c1)
        .chain(p2, c2);
IntStream.range(0, 10).boxed().forEach(routerMatchAll.toConsumer());
System.out.println(">>>>>>>>>>>>>>>>>>>>>>");
Router<Integer> routerMatchOnce = RouterMatchOnce.of(p1, c1).chain(p2,
        c2);
IntStream.range(0, 10).boxed().forEach(routerMatchOnce.toConsumer());

How about a system where you would implement the first stream:

Router<Integer> routerMatchAll = RouterMatchAll.of(p1, c1)
        .chain(p2, c2);
IntStream.range(0, 10).boxed().forEach(routerMatchAll.toConsumer());

as:

    IntStream.range(0, 10).boxed()
        .peek(new ConditionalTee<>(p1, c1))
        .forEach(new ConditionalTee<>(p2, c2));

Note how you use peek() to ensure all values pass through to the forEach()?

The second implementation is currently:

Router<Integer> routerMatchOnce = RouterMatchOnce.of(p1, c1).chain(p2,
        c2);
IntStream.range(0, 10).boxed().forEach(routerMatchOnce.toConsumer());

but I would instead suggest:

    IntStream.range(0, 10).boxed()
        .filter(new ConditionalTee<>(p1, c1))
        .forEach(new ConditionalTee<>(p2, c2));

Note how we call filter() instead of peek()?

Now, what is ConditionalTee? It is a simple class that takes a predicate, and a Consumer, and it calls the consumer with the value, if the value passes, and it returns false if the predicate passes (which means the value will be filtered in a filter call). It is more comments than code, and it's really simple:

public class ConditionalTee<T> implements Predicate<T>, Consumer<T> {

    private final Predicate<? super T> predicate;
    private final Consumer<? super T> consumer;

    public ConditionalTee(
          Predicate<? super T> condition, Consumer<? super T> consumer) {
        this.predicate = condition;
        this.consumer = consumer;
    }

    @Override
    public void accept(T t) {
        test(t);
    }

    @Override
    public boolean test(T t) {
        // NOTE: inverted boolean.
        // this will be called from a Stream.filter()
        // need to return true if the value should stay on the stream
        // false if it should be removed from the stream.
        // We want to keep it on the stream if we are not siphoning it off
        //     (return true).
        // We want to remove from the stream if we are siphoning the value off
        //     (return false).
        if (!predicate.test(t)) {
            return true;
        }
        consumer.accept(t);
        return false;
    }
}

See it all running here, in ideone.


React breadcrumbs component implementation I've written a breadcrumbs component (RouteBreadcrumbs) that has the following features/behavior:

Takes in an array of labels for each breadcrumb (ex: ['Settings', 'System settings', 'Display']. The component displays these labels in
the order that they're placed in in the array.
Assumes that the last label in the array represents the currently viewed page. For the rest of the labels that come before it, the user
can click on one in order to navigate to that page.
Gets the current path it is rendered in and parses it in order to assign the correct path for each breadcrumb (except the last one)
If the current path does not contain enough "subpaths" to satisfy the number of labels passed in, I've chosen not to render the component. For example, the component would not render if the current path was only /system-settings/display when labels contained three items.

Ultimately, the component would look like this when rendered (I haven't yet implemented the arrows between each breadcrumb):
Settings > System settings > Display

In this scenario, "Display" would be the currently viewed page. And the user would be able to click on "Settings" or "System settings" in order to jump to those respective pages.
I'd love to get feedback on my implementation. I'm also interested in learning more about best coding practices and whether there are any strong code smells in my component (and if there is one, why it is considered a code  smell).
Some areas that I think might be code smells but am not really sure if they are or why they are:

Calling pathNameIdx-- both inside the while loop and then again once the loop is exited
Having a return statement in multiple places in my component (in my case, in just two places)
Using a regular for loop instead of other forms like for..of, for..in, or .forEach.

import React from "react";
import { useHistory, useLocation } from "react-router-dom";

const RouteBreadcrumbs = ({labels = []}) => {
    const location = useLocation();
    const history = useHistory();
    const pathName = location.pathname;

    const numBackslash = (pathName.match(/\//g) || []).length;
    if (numBackslash < labels.length) {
        return null;
    }

    let breadcrumbPaths = [];
    let pathNameIdx = pathName.length - 1;
    for (let i = labels.length - 1; i > 0 ; i--) {
        while ((pathName[pathNameIdx] !== "/")) {
            console.assert(pathNameIdx >= 0, "pathNameIdx is no longer a valid index");
            pathNameIdx--;
        }

        const newPath = pathName.substring(0, pathNameIdx);
        breadcrumbPaths = [newPath, ...breadcrumbPaths];
        pathNameIdx--;
    }

    return (

            {labels.map((label, idx) => <h5 key={idx} onClick={idx < breadcrumbPaths.length ? () => history.push(breadcrumbPaths[idx]) : null} className={"label2"}>{label}
)}

    )
};

```

Your code is looking pretty good. Here's a handful of suggestions you could try:
This part
{labels.map((label, idx) => <h5 key={idx} onClick={idx < breadcrumbPaths.length ? () => history.push(breadcrumbPaths[idx]) : null} className={"label2"}>{label}
)}

is a really long line of code. Try breaking it up, it makes it easier to read.
{labels.map((label, idx) => (
  <h5
    key={idx}
    onClick={
      idx < breadcrumbPaths.length
        ? () => history.push(breadcrumbPaths[idx])
        : null
    }
    className={"label2"}
  >
    {label}

))}

(I tend to prefer 2-space indentation with javascript and HTML as they both tend to be indentation heavy, and it seems to be a more common in the javascript community - so I'll use that in these examples)
Also, refrain from using an index for an element key when possible (the react docs talk about why). You could instead use the breadcrumbPath, as that would be unique to each element, and the keys would correctly change when the path changes.
{labels.map((label, idx) => (

    {label}

))}

You have some logic in there to calculate the number of backslashes a URL contains, but if you just wait a little bit on that early return, you will already have easy access to that number, as its the same as breadcrumbsPath.length + 1
if (breadcrumbPaths.length + 1 < labels.length) {
  return null;
}

On that note, this function won't handle the case correctly when there's a leading slash on the URL (i.e. 'example.com/a/b/') - that'll need to be taken care of. If you use the above code snippet, then you will only need to fix this issue in one place instead of two - where you create the breadcrumbPaths array.
To address your concern of having multiple returns: there's nothing wrong with having multiple returns like that in a function - in fact, it can often lead to cleaner code. Often, the alternative means large if-thens or flag variables, both of which are much harder to follow. I've heard that once upon a time a function with a single exit used to be something encouraged, but that's not the case anymore.
Now onto the part that you probably care about most - what to do about that chunk of logic with the c-style for loop, and multiple pathNameIdx--s.
First things first, I would probably break the meat of that logic out into a helper function. There's a lot of low-level work going on there that gets in the way of reading and understanding your component.
Here's how your component might look after doing that
import React from "react";
import { useHistory, useLocation } from "react-router-dom";

const RouteBreadcrumbs = ({labels = []}) => {
  const location = useLocation();
  const history = useHistory();
  const pathName = location.pathname;

  const breadcrumbPaths = ancestorPaths(pathName);
  if (breadcrumbPaths.length + 1 < labels.length) {
    return null;
  }

  return (

      {labels.map((label, idx) => (
        <h5
          key={breadcrumbPaths[idx]}
          onClick={
            idx < breadcrumbPaths.length
              ? () => history.push(breadcrumbPaths[idx])
              : null
          }
          className={"label2"}
        >
          {label}

      ))}

  )
};

function ancestorPaths(pathName) {
  let breadcrumbPaths = [];
  let pathNameIdx = pathName.length - 1;
  while (true) {
    while (pathName[pathNameIdx] !== "/") {
      if (pathNameIdx === 1) {
        return breadcrumbPaths;
      }
      pathNameIdx--;
    }

    const newPath = pathName.slice(0, pathNameIdx);
    breadcrumbPaths = [newPath, ...breadcrumbPaths];
    pathNameIdx--;
  }
}

Notice how there's a lot less low-level logic going on in the actual RouteBreadcrumbs component? It's a lot easier to read through it and get a good idea of what's going on - except for maybe the actual jsx getting returned - it's a little too nested for my liking, but it'll get simplified in a bit.
A couple other things I'll note with your original code:

Prefer string.slice() over string.substring() - it has fewer quarks
Good use of console.assert() there. The one thing I dislike about console.assert() is that it doesn't actually throw an error, it just logs one. Sometimes this isn't a big deal, here it is. If that assertion happened to come true, it would be stuck in an infinite loop, logging forever. You can just do if (condition) throw new Error(...) instead, or make your own assert function that actually throws an error.

Now lets look at simplifying our new ancestorPaths() helper function. It is possible to use regex to gather the indicies of all the slashes, which would work similar to how you have it, but greatly simplify the logic (see here). I'm going to instead just split on '/', build a list, and use a generator function, as follows:
function* iterAncestorPaths(pathName) {
  const parts = pathName.split("/").filter(x => x !== "");
  for (let i = 1; i < parts.length; ++i) {
    yield "/" + parts.slice(0, i).join("/");
  }
}

Note that the .filter() at the beginning will take care of both the leading / and a potential trailing slash.
One final thing: If your breadcrumb segments are links, why not make them actual links? it'll simplify some things.
Here's the final solution, with a couple other misc tweaks.
import React from "react";
import { Link, useLocation } from "react-router-dom";

const RouteBreadcrumbs = ({labels = []}) => {
  const pathName = useLocation().pathname;

  const ancestorPaths = [...iterAncestorPaths(pathName)];
  if (ancestorPaths.length + 1 < labels.length) {
    return null;
  }

  return (

      {ancestorPaths.map((path, idx) => (
        <Link key={path} to={path} className="label">
          {labels[idx]}
        </Link>
      ))}
      {labels[labels.length - 1]}

  );
};

function* iterAncestorPaths(pathName) {
  const parts = pathName.split("/").filter(x => x !== "");
  for (let i = 1; i < parts.length; ++i) {
    yield "/" + parts.slice(0, i).join("/");
  }
}


Function to check truth value of a specific statement I want to check the truth of A + B - (A & B) == A | B for N-bit numbers. For example, here's a "truth" table for 2-bit numbers. (I know I'm taking liberty with the concept of a "truth" table.)

Here's the simple code I threw together in Python.

def check_truth(bits):

    for x in range(2**bits):
        for y in range(bits*2):
            if x + y - (x & y) != x | y:
                return "False for {} bits".format(bits)

    return "True for {} bits".format(bits)

It spits out the correct answer, but it's obviously really inefficient. I am not good at algorithm analysis, but it's something like \$O(2^N)\$ right?

It's pretty much just a one-time-use function, but I'm curious how it could be improved. For example, I figure you could skip over number pairs that had been tested before. Any other ideas?

Make the function return True or False.
You can use all and a generator comprehension instead of returning early.
You should make the range have the same input.

Putting these together you can get:

def check_truth(bits):
    return all(
        x + y == (x | y) + (x & y)
        for x in range(bits ** 2)
        for y in range(bits ** 2)
    )

print("{} for {} bits".format(check_truth(bits), bits))

The simplest way to prove:

$$x + y = (x \land y) + (x \lor y)$$

Is to think of integers as a bunch of wires that are passed to an infinite full adder. Due to how full adders work, they will not care how the bits are placed, as it makes no logical difference to its functionality.
So \$010 + 101 = 111 + 000\$, which means as long as we know there are the same amount of bits on each section of the equation, then we know they are the same.
Since we know \$a = x \lor y\$ and \$b = x \land y\$, we can test if there are any missing or new bits.
And so using the following 'table' shows that each level will have the same amount of bits:

$$
x, y, a, b\\
0, 0, 0, 0\\
1, 0, 1, 0\\
0, 1, 1, 0\\
1, 1, 1, 1\\
$$

And so to get the best performance we can re-write check_truth:

def check_truth(bits):
    return True

To make a more compelling argument, we could sub \$a\$ and \$b\$ into a full adder equation, and show there's no difference, but if you think about this logically you should notice it's always true.

$$
S = C \oplus (A \oplus B)\\
C_1 = ((A \oplus B) \land C) \lor (A \land B)
$$

Sub \$A = x \lor y\$ and \$B = x \land y\$:

$$
S = C \oplus ((x \lor y) \oplus (x \land y))\\
C_1 = (((x \lor y) \oplus (x \land y)) \land C) \lor ((x \lor y) \land (x \land y))
$$

Simplify some things:

$$
(x \lor y) \oplus (x \land y) = x \oplus y\\
(x \lor y) \land (x \land y) = x \land y
$$

Sub simplified versions into main function:

$$
S = C \oplus (x \oplus y)\\
C_1 = ((x \oplus y) \land C) \lor (x \land y)
$$

See that it works the same way.


Unit Testing and good use of matchers I been having discussion with colleagues recently about good use of matchers. Consider the following sample:

@RunWith(MockitoJUnitRunner.class)
public class LoginRestServiceImplTest {

    @Mock
    private LoginService loginService;

    @InjectMocks
    private LoginRestService loginRestService = new LoginRestServiceImpl();

    private LoginRequest loginRequest;
    private LoginResponse loginResponse;

    @Before
    public void setUp() throws Exception {
        loginRequest = new LoginRequest("Username","Password");
    }

    @Test
    public void thatUserCanLoginWhenUserAndPassCorrect() {
        // Given
        loginResponse = new LoginResponse();
        loginResponse.setSuccess(true);

        when(loginService.login(anyString(), anyString())).thenReturn(loginResponse);

        // When
        LoginResponse result = loginRestService.login(loginRequest);

        // Then
        assertTrue("User Can login", result.isSuccess());
    }

    @Test
    public void thatUserCantLoginWhenPasswordIncorrect() {
        // Given
        loginResponse = new LoginResponse();
        loginResponse.setSuccess(false);
        loginResponse.setError(INCORRECT_PASSWORD);

        when(loginService.login(anyString(), anyString())).thenReturn(loginResponse);

        // When
        LoginResponse result = loginRestService.login(loginRequest);

        // Then
        assertFalse("User Can't login", result.isSuccess());
        assertEqual("Error Code is Incorrect Password", INCORRECT_PASSWORD, loginResponse.getError());
    }
}

Note this is just a sample. Normally we would use a spring MVC for unit testing a REST service.
But is the use of anyString() valid. Does it mean we are being too broad with the mocking criteria?

Is there anything else that is wrong with this style?

Well, what you do not actually test is: Are the login parameters passed to the LoginService correctly? In your example, you are simply assuming that the next login will fail or succeed, but you do that completely ignoring the actual parameters.

Especially for the successfull test I would use the actual expected parameters instead of anyString. For the not successfull test you could do the same, return a result with false if the login data is the expected value (otherwise it will be null).

This way you don't just test the way the LoginRestService handles the responses from the LoginService, but also that the LoginRestService gives the correct parameters to the LoginService in the first place, eliminating one possible cause of errors.


Drawing Routes on a Map I have this class:

public class MapController
{
    private Polyline _currentPolyline; // The line connecting two locations on the map
    private GoogleMap _map;
    private Context _context;

    public MapController(GoogleMap map, Context context)
    {
        _map = map;
        _context = context;
    }

    /**
     * Draws a route on the map.
     *
     * @param directions the Google Map Directions JSON string with the routes
     * @throws JSONException
     */
    public void drawRoute(String directions) throws JSONException
    {
        // First parse the JSON to extract the points into an easily manipulated form
        List<List<HashMap<String, String>>> routes = new DirectionsHelper().parsePathsFromJson(directions);

        // Only one polyline can be shown on the map at a time
        hideRoute();

        ArrayList<LatLng> points;
        PolylineOptions polyLineOptions = null;

        // Traversing through routes
        for (int i = 0; i < routes.size(); i++)
        {
            points = new ArrayList<LatLng>();
            polyLineOptions = new PolylineOptions();
            List<HashMap<String, String>> path = routes.get(i);

            for (int j = 0; j < path.size(); j++)
            {
                HashMap<String, String> point = path.get(j);

                double lat = Double.parseDouble(point.get("lat"));
                double lng = Double.parseDouble(point.get("lng"));
                LatLng position = new LatLng(lat, lng);

                points.add(position);
            }

            polyLineOptions.addAll(points);
            polyLineOptions.color(ContextCompat.getColor(_context, R.color.colorPrimary));
        }

        _currentPolyline =  _map.addPolyline(polyLineOptions);
    }

    /**
     * Removes the currently drawn {@link Polyline} from the map.
     */
    public void hideRoute()
    {
        if (_currentPolyline != null)
        {
            _currentPolyline.removeRoute();
        }
    }
}

Is it generally a good practice to call hideRoute() inside the drawRoute() method or does it depend on other factors? For example: should the caller be responsible for clearing the map before drawing the new path or does the caller not care?

From an algorithm flow viewpoint, it looks good. However, I'd make a few small changes:

public void drawRoute(String directions) throws JSONException
{
    // First parse the JSON to extract the points into an easily manipulated form
    List<List<HashMap<String, String>>> routes = new DirectionsHelper().parsePathsFromJson(directions);

    // Only one polyline can be shown on the map at a time
    hideRoute();

    ArrayList<LatLng> points;
    PolylineOptions polyLineOptions = null;

    // Traversing through routes
    for (int i = 0; i < routes.size(); i++)
    {
        points = new ArrayList<LatLng>();
        polyLineOptions = new PolylineOptions();
        List<HashMap<String, String>> path = routes.get(i);

        for (int j = 0; j < path.size(); j++)
        {
            HashMap<String, String> point = path.get(j);

            double lat = Double.parseDouble(point.get("lat"));
            double lng = Double.parseDouble(point.get("lng"));
            LatLng position = new LatLng(lat, lng);

            points.add(position);
        }

        polyLineOptions.addAll(points);
        polyLineOptions.color(ContextCompat.getColor(_context, R.color.colorPrimary));
    }

    _currentPolyline =  _map.addPolyline(polyLineOptions);
}

In drawRoute, you first declare polyLineOptions = new PolylineOptions();, and then after the for loop, you assign values.

I think it'd be better if you moved the creation of PolylineOptions so that it's created and used in one place:

 public void drawRoute(String directions) throws JSONException
{
    // First parse the JSON to extract the points into an easily manipulated form
    List<List<HashMap<String, String>>> routes = new DirectionsHelper().parsePathsFromJson(directions);

    // Only one polyline can be shown on the map at a time
    hideRoute();

    ArrayList<LatLng> points;
    PolylineOptions polyLineOptions = null;

    // Traversing through routes
    for (int i = 0; i < routes.size(); i++)
    {
        points = new ArrayList<LatLng>();
        List<HashMap<String, String>> path = routes.get(i);

        for (int j = 0; j < path.size(); j++)
        {
            HashMap<String, String> point = path.get(j);

            double lat = Double.parseDouble(point.get("lat"));
            double lng = Double.parseDouble(point.get("lng"));
            LatLng position = new LatLng(lat, lng);

            points.add(position);
        }

        polyLineOptions = new PolylineOptions();
        polyLineOptions.addAll(points);
        polyLineOptions.color(ContextCompat.getColor(_context, R.color.colorPrimary));
    }

    _currentPolyline =  _map.addPolyline(polyLineOptions);
}

And for the hideRoute function,

public void hideRoute()
{
    if (_currentPolyline != null)
    {
        _currentPolyline.removeRoute();
    }
}

You should set _currentPolyline to null so you can't remove it twice (if that matters), plus it allows the GC to clean up the polyline.


Effieciently validate multiple DataRows with return early option I'm looking for an efficient way to validate multiple rows originating from multiple .csv files. Below is the current code to accomplish that.
The validation logic is in the function ValidateRow(DataRow row) (not presented here).

I could think of an alternative with using Linq and performing batch.All( row => ValidateRow(row)) and adding the row to the InvalidRow property in ValidateRow. But I don't know if that is more efficient or not.

public bool ValidateBatch( IEnumerable<DataRow> batch, bool exitOnFirstError = false )
    {
      //return early if we find no datarows
      if( batch == null || batch?.Count() == 0 ) { logger.Error( MSG_NO_DATAROWS ); return false; }
      bool result = true;

      foreach( DataRow row in batch )
      {
        if( !ValidateRow( row )  )
        {
          result = false;
          // track invalid rows on a property (List<DataRow>)
          InvalidRows.Add( row );
          if( exitOnFirstError )
          {
            return result;
          }
        }
      }
      return result;
    }

The if condition

if( batch == null || batch?.Count() == 0 ) { logger.Error( MSG_NO_DATAROWS ); return false; }

has some flaws:

the Null-Conditional operator ? on batch?.Count() == 0 isn't needed, because you already check for batch == null
If the passed IEnumerable<DataRow> isn't a Collection<T> the IEnumerable<DataRow> will be enumerated completely for calculating the Count.
Assume you have millions of items in that IEnumerable<T> guess how much time it takes to iterate twice (once for Count() and once for the foreach).
Having multiple statements in a single line reduces the readability of the code.

A better way would be to use the Any() method like so

if( batch == null || !batch.Any())
{
    logger.Error( MSG_NO_DATAROWS );
    return false;
}


Disposable resource: lazy allocation Background

I'm writing a wrapper for a significant portion of a native library in Java. The native objects my classes are wrapping are giving me some headaches in terms of design.

I'm not a fan of the two-step initialization idiom but I've resigned myself to use it if the native object has a lot of configuration options followed by a rather complex and error-prone initialization.

The real issue is that I still have some constructors that throw exceptions after native resources have been allocated, though I'm able to avoid it in most cases. All classes wrapping native resources implement the Disposable interface.

public interface Disposable extends AutoCloseable
{
   boolean isClosed();

   @Override
   void close();
}

The close method is always written in the most safest way, making no assumptions about the state of the object. Okay, so I can call it from the constructor... that is unless there are subclasses that override this method.

Code

So I've been thinking about an elegant solution to solve these problems. I've decided to upgrade the allocation and deallocation of native objects to full object status.

public interface Resource<T> extends Disposable
{
    T get();
}

These Resource objects are supposed to be pushed onto a stack that is shared with every class in the hierarchy so there is no longer a need to override the close method of a base class.

I have an abstract implementation that lazily allocates the native object once and returns it. I cannot use the double checked locking idiom to do this because I'm dealing with three states.

Either the native object hasn't been allocated, either it has, or it has been deallocated. If it has been deallocated, I don't want the possibility that it may accidentally be reallocated.

public abstract class AbstractResource<T> implements Resource<T>
{
    private enum State
    {
        INITIAL,
        ALLOCATED,
        DISPOSED
    }

    private final AtomicReference<State> refState;
    private final AtomicReference<T> refResource;

    public AbstractResource()
    {
        this.refState = new AtomicReference<>(State.INITIAL);
        this.refResource = new AtomicReference<>(null);
    }

    @Override
    public boolean isClosed() {
        return this.refState.get() == State.DISPOSED;
    }

    @Override
    public T get()
    {
        final State currentState = this.refState.get();
        T resource = this.refResource.get();

        if (currentState == State.DISPOSED)
            throw new ObjectDisposedException(resource.getClass().getName());

        if (currentState == State.INITIAL) {
            if (this.refState.compareAndSet(State.INITIAL, State.ALLOCATED)) {
                try {
                    resource = this.allocate();
                    this.refResource.set(resource);
                } catch (Exception ex) {
                    this.refState.set(State.INITIAL);
                    throw ex;
                }
            }
        }
        return resource;
    }

    protected abstract T allocate();

    @Override
    public void close()
    {
        if (this.refState.getAndSet(State.DISPOSED) == State.ALLOCATED)
            this.release(this.refResource.get());
    }

    protected abstract void release(final T resource);
}

Is this class 100% thread-safe or have I missed anything? Might there be a better way to do this that does not require locking or at least keep it to a minimum?

I'm not a fan of the two-step initialization idiom but I've resigned myself to use it if the native object has a lot of configuration options followed by a rather complex and error-prone initialization.
The real issue is that I still have some constructors that throw exceptions after native resources have been allocated, though I'm able to avoid it in most cases.

That's a suspicious set of problems to have; too bad you didn't ask for a review of that code.  Off the top of my head, however, I've generally found that "a lot of configuration options" can usually be managed by a builder, and that trivial constructors seldom throw -- dependency injection pushes the exception risk outside of the constructor, where you can usually manage things more easily.
@Override
public void close()

If you aren't intending to let people create sub classes that circumvent the state machine, you might make these methods final.
protected abstract T allocate();
protected abstract void release(final T resource);

These methods don't belong here.  The rest of your class is about lifecycle management; these are about resource management.  You've mixed the concern about coordinating the life cycle with your concern for creation.
interface ResourceFactory<T> {
    void T allocate();
    void release(T resource);
}

Then, in your lifecycle management, defer the work to the factory.
@Override
public void close()
{
    if (this.refState.getAndSet(State.DISPOSED) == State.ALLOCATED)
        this.factory.release(this.resource);
}

10 out of 10 for making the state machine explicit.  I suspect that you should make State.ALLOCATING and State.DISPOSING explicit.  You might also want to look into Stateless4j; trivial state machines often grow in complexity.


Parse user data in to student and teacher details Initially I have a code in JavaScript which looks like -
const createUsersInfo = obj => {
    const studentDetails = [];
    const teacherDetails = [];
    obj.forEach(x => {
      if (x.type) {
        if ( x.type === 'teacher' && x.group !== 'admin' ) {
            teacherDetails.push(x);
        } else if ( x.type === 'student' && x.group !== 'admin' ) {
            studentDetails.push(x);
        }
      }
    });

    return {
        studentDetails,
        teacherDetails
    };
};

Then, I refactored it to -
const createUsersInfo = obj => {
  const studentDetails = [];
  const teacherDetails = [];
  obj.forEach(x => {
    const type = x.type;
    if ( type && x.group !== 'admin' ) {
      if ( type === 'teacher' ) {
        teacherDetails.push(x);
      }
      if ( type === 'student' ) {
        studentDetails.push(x);
      }
    }
  });

  return {
    studentDetails,
    teacherDetails
  };
};

Is this better as compared to my initial version or it can  further improved ?
Thanks.

A short review;

You can avoid process typeless and admin users by using an upfront .filter()
obj is a terrible name (it implies the parameter is just an Object instead of an Array, list would be better, something meaningful like userDetails the best
If you want to go for slightly less efficient code, you could replace the whole loop with 2 filters
Dont go for fat arrow syntax unless you are coding and inline function

I would go for something like this;
function createUsersInfo(userDetails){
  const cleanUserDetails = userDetails.filter(user => (user.type && user.group != 'admin'));
  return {
    studentDetails: cleanUserDetails.filter(user => user.type === 'student'),
    teacherDetails: cleanUserDetails.filter(user => user.type === 'teacher')
  };
}


Is this FTP download code useful? I have a Windows-service and this service will connect to an ftp server and do download, upload, and rename operations.

My service runs every 30 seconds, so the ftp server will be busy during these operations.

My question is: is my code useful for this job? Because my download/upload/rename functions open a webrequest each process? I use FtpWebRequest and FtpWebResponse

I write only the download functions, the other functions are similar.

Download function:

    public Result Download(string localPath,string remotePath,string fileName)
    {
        Result _result = new Result();
        FtpWebRequest requestFileDownload = null;
        FtpWebResponse responseFileDownload = null;

        try
        {

            //
            requestFileDownload =
                (FtpWebRequest)WebRequest.Create(remotePath + fileName);
            //
            requestFileDownload.Credentials =
                new NetworkCredential(this.Username, this.Password);
            //
            requestFileDownload.Method =
                WebRequestMethods.Ftp.DownloadFile;

            responseFileDownload =
                (FtpWebResponse)requestFileDownload.GetResponse();

            using (Stream responseStream = responseFileDownload.GetResponseStream())
            {
                FileStream writeStream = new FileStream(localPath + fileName, FileMode.Create);

                int Length = 2048;
                Byte[] buffer = new Byte[Length];

               int bytesRead;
               while((bytesRead = responseStream.Read(buffer, 0, Length)) > 0)
               {
                  writeStream.Write(buffer, 0, bytesRead);
               }

                writeStream.Close();

            }

            return _result;

        }
        catch (Exception x)
        {
            _result.Error = true;
            _result.ErrorMessage = x.StackTrace;
            _result.ErrorFileName = fileName;
            _result.ErrorFilePath = remotePath + fileName;

            return _result;
        }
        finally
        {
            requestFileDownload = null;
            responseFileDownload = null;

        }

    }

    public struct Result
    {
        public bool Error;
        public string ErrorMessage;
        public string ErrorFileName;
        public string ErrorFilePath;
        public List<string> Files;
    }

A few things:

I would supply all the variables that you have hard coded into the method i.e. localPath, filename, ftp server, networkcredentials.  Otherwise how can you re-use this method for other operations?
I would probably avoid making the method static unless there was good reason to.
In your finally clause you don't check if responseStream or writeStream are null?  What if they never got created before an exception was thrown?
I personally think unless there is a very valid reason you should always do something in an exception catch block, whether it be logging somewhere, re-throwing the exception or anything.  Otherwise the calling method will have no idea that something went wrong.
What is _result?  I don't see it being set and I don't see it being declared anywhere?
If Length is a constant, make it a constant.  i.e. const int Length = 2048.
I might also consider implementing some sort of call back method so you can provide status updates on the progress on the download.  Just a suggestion, not sure how you would implement it exactly.

I might consider writing the loop as such:

int bytesRead;
while((bytesRead = responseStream.Read(buffer, 0, Length)) > 0)
{
   writeStream.Write(buffer, 0, bytesRead);
}

As to your quesion.  Is this code useful?  In short.  Yes, but only if you ever want to do only one thing with it and never want to re-use anything inside.  Otherwise no.

That's just a few comments off the top of my head.  I hope those are a good starting point.


Algorithm that parses through input of points and finds distance I'm wondering if someone could take some time to review this script. I'm parsing through a list of points of any length and calculating the distance. I'm wondering how to make my code better/more efficient (something I'm working on). A sample input file would be like so:

300.754178236262248 103.453277023380423 0,276.62980277988612 90.123295023340319 0,269.345711570634421 103.319531391674346 0,293.447811515317824 116.649513392506364 0,300.754178236262248 103.453277023380423 0

I'm not sure why the zeros are there; these are from the spacenet label csv's.

And this is my code:

import math

def calc(x1, y1, x2, y2):
    dist = math.sqrt((x2-x1)**2 + (y2-y1)**2)
    return dist

li = []
res = []
with open("/Users/jin/points.txt") as filestream:
    for line in filestream:
        temp = line.split(",") #splits by the comma in a single list

        for i in temp:
            temp = i.split(" ") #splits by spaces to individual lists of points
            li.append(temp) #list of lists containing each point
    # for item in li:
    #   x1 = item[1]
    #   y1 = item[0]

    # for item in li:
    #   for pt in item:
    #       print pt

    for index in range(len(li)-1):
        one = li[index]
        two = li[index+1]
        x1 = float(one[0])
        y1 = float(one[1])
        x2 = float(two[0])
        y2 = float(two[1])
        res.append(calc(x1, y1, x2, y2))
    print res

Choose a better name than calc(). Something like distance() or euclidean_distance().
There is no need for the variable temp, just write
for i in line.split(",")
The for i in temp loop can be written more simply as:

li.extend(i.split() for i in temp)

Remove all commented out code that you don't immediately need.
The for index in range(len(li)-1): doesn't need to be in the with statement, as it is not using filestream.
res can be calculated as part of the for i in temp: loop and avoid li altogether.


Overlaping sections using Bootstrap and CSS I'm currently converting a PSD to a web template and attempting to overlap content and I'm running into a issue with the 3rd section because there is  more padding than the first overlap and using the CSS style background-color covers wherever there is a block element but if there isn't anything in the pace a image takes priority.

I have a link to the github page where it is live in it's current state

The phone image is above the background color of the next section, I've tried using z-index but it still seems to take priority over the next section. Does anyone know a css trick or the proper method to do this overlap like in the previous section I've overlapped the content.

Here is some of the HTML code

DESIGN

Lorem ipsum dolor sit amet, consectetur adipisicing elit. A amet assume consequatur deleniti eligendi eos, expedita fugit maxime, natus nostrumas officia officiis quisquam quos repellendus verit verodasdfudgera voluptate. Nihil, perspiciatis puth.

                    <button class="button" type="submit">GET STARTED</button>

01/05

                     &nbsp; &nbsp;

OUR COMMUNITY

Lorem ipsum dolor sit amet, consectetur adipisicing elit.

The CSS

/* ================== SECOND SECTION =================== */

.second-section {
    background-color: #FFF;
    margin-top: -300px;
}
.second-section .content {
    background-color: #FFF;
    padding-top: 150px;
}

.second-section h3 {
    word-spacing: 5px;
    letter-spacing: 2px;
    line-height: 1.25em;
    color: #16a085;
}

.second-section img {
    z-index: -10;
}

.second-section p {
    font-size: 1em;
    line-height: 2em;
}

.second-section p:last-of-type {
    padding-top: 50px;

}
.second-section i {
    font-size: 2em;
    font-weight: bold;
}

/* ===========================  THIRD SECTION =========================== */

.third-section {
    background-color: #f5f6f7;
    margin-top: -150px;
    padding-top: 150px;
    z-index: 10;
}
.third-section .content {
    background-color: #f5f6f7;
}

.content {
    background-color: #f5f6f7;

}

Essentially I'm just using margin to pull the next section up and overlap the previous section. If someone knows of a better way that's utilizing bootstrap or pure CSS that would be great but i've tried several different things.

Shortly after the post I found out that also adding a background color to individual block elements and applying padding to both top & bottom will actually stretch out the background-color and allow of the use of margins to cover other elements like so

.grayBG {
    background-color: #f5f6f7;
    padding-top: 100px;
    padding-bottom: 100px;
}

and

.whiteBG {
background-color: #FFF;
padding-top: 150px;

}

then with the use of a -margin I was able to pull sections to overlap each other like so.

.second-section {
background-color: #FFF;
margin-top: -300px;
padding-bottom: 0;

}

There is probably a better and more efficient way of doing this but, this seemed to work for me just fine.


My first RSpec test - Rock Paper Scissors I'm relatively familiar to Ruby, but I'm brand-new to testing code. I've decided to use RSpec because as it seems like a popular option.

The game works perfectly, but the spec has a lot of repetition and I have a feeling it can be improved.

The Spec

require 'rock_paper_scissors'

# game = RockPaperScissors.new('rock')
# game.result => ['win!', 'lose!', 'tie!']
#
# 1 == rock
# 2 == paper
# 3 == scissors

describe "Rock Paper Scissors" do
  let(:answers_snippet) { /valid answers/ }

  context "when a number is passed" do

    it "fails when invalid" do
      game = RockPaperScissors.new(0)
      game.result.should match(answers_snippet)
    end

    it "passes when valid and convert to string value" do
      game = RockPaperScissors.new(1)

      game.result.should_not match(answers_snippet)
      game.choice.should == "rock"
    end
  end

  context "when a string is passed" do

    it "fails when invalid" do
      game = RockPaperScissors.new('cheese')
      game.result.should match(answers_snippet)
    end

    it "passes when valid" do
      game = RockPaperScissors.new('rock')
      game.result.should_not match(answers_snippet)
    end
  end

  it "should tie when rock vs rock" do
    RockPaperScissors.any_instance.stub(:random_choice).and_return('rock')

    game = RockPaperScissors.new('rock')
    game.result.should == "tie!"
  end

  it "rock should beat scissors" do
    RockPaperScissors.any_instance.stub(:random_choice).and_return('scissors')

    game = RockPaperScissors.new('rock')
    game.result.should == "won!"
  end

  it "paper should beat rock" do
    RockPaperScissors.any_instance.stub(:random_choice).and_return('rock')

    game = RockPaperScissors.new('paper')
    game.result.should == "won!"
  end

  it "scissors should beat paper" do
    RockPaperScissors.any_instance.stub(:random_choice).and_return('paper')

    game = RockPaperScissors.new('scissors')
    game.result.should == "won!"
  end

end

I've created a gist that has both the Rock Paper Scissors class and RSpec tests.

let(:answers_snippet) { /valid answers/ }

it "fails when invalid" do
  game = RockPaperScissors.new(0)
  game.result.should match(answers_snippet)
end

When I read this spec code, I see that result should match valid answers, but from the spec name I see, that we test for invalid answers. Your specs code should be easy readable and I recommend to name constants correctly:

let(:invalid_answers_pattern) { /invalid answer/ }

Your specs are small and independent and if one spec will fail, it is enough to read this spec's code to understand how to reproduce problem. Specs are fine. But I would removed duplication in the last 4 specs, I think it won't hurt specs readability:

test_round_result(your_choice, opponent_choice, expected_result) do
  RockPaperScissors.any_instance.stub(:random_choice).and_return(opponent_choice)

  game = RockPaperScissors.new(your_choice)
  game.result.should == expected_result
end

it "should tie when rock vs rock" do
  test_round_result( 'rock', 'rock', 'tie!' )
end

#...

Also I found that you do too much logic in the initializer of your game class:

def initialize(choice)
    @choice = choice

    if is_valid_choice?
      @result = play
    else
      @result = instructions
    end
end

Class initializer should accept parameters and make sure your class instance is in the consistent state (is_valid_choice? in your case). You can write your class initializer this way:

def initialize(choice)
    raise ArgumentError, instructions unless valid_choice?(choice)
    @choice = choice
end

Or even accepted choice in the play method because choice is a parameter for the round of the game, not for the whole game and leave initializer empty for now. When you will start doing BDD, you will likely star making design decisions like this.
With this refactoring your tests will be a little bit cleaner, something like this:

let(:game) { RockPaperScissors.new() }

test_round_result(your_choice, opponent_choice, expected_result) do
  game.stub(:random_choice).and_return(opponent_choice)
  game.play(your_choice).should == expected_result
end


Construct date sequence in Scala I want to have a continuous date sequence like ['2014-01-01','2014-01-02', ...]

Then I define a stream to do that.

def daySeq(start: Date): Stream[Date] = Stream.cons(start,  daySeq(plusDays(start, 1)))

I get the sequence within range [start, end) by calling

daySeq(from).takeWhile(_.getTime < to.getTime)

Any better/simple solution to do this ?

I suggest using Joda-Time's LocalDate instead of Java's Date to represent dates without a time zone.

Assuming you only need to traverse the days once, use an Iterator instead of a Stream.

def dayIterator(start: LocalDate, end: LocalDate) = Iterator.iterate(start)(_ plusDays 1) takeWhile (_ isBefore end)

Example usage:

dayIterator(new LocalDate("2013-10-01"), new LocalDate("2014-01-30")).foreach(println)

If you do need a lazily-evaluated list, then Stream is appropriate. I suggest using iterate instead of cons in that case.

def dayStream(start: LocalDate, end: LocalDate) = Stream.iterate(start)(_ plusDays 1) takeWhile (_ isBefore end)


Splitting a sequence into equal segments I need to split up a sequence into equal segments of a given size (yes, the last one may be shorter), and I'm trying to find an efficient and idiomatic way to do it.

I have two versions of the function; segment is the F# version of an awkward and probably quite inefficient old C# extension method that I wrote, while chop is an attempt to do it in a more intelligent manner, but is still rather imperative. I would have liked to have a solution that doesn't need to use reference cells, but could not come up with one.

module Seq =
    let segment segmentSize source =
        seq {
                let segmentCount = ref 0
                while !segmentCount * segmentSize < Seq.length source
                    do
                        let segment = source |> Seq.skip (!segmentCount * segmentSize) |> Seq.truncate segmentSize
                        segmentCount := !segmentCount + 1
                        yield segment
        }

    let chop segmentSize source =
        seq {
                let chopSource = ref source
                while not <| Seq.isEmpty !chopSource
                    do
                        let segment = !chopSource |> Seq.truncate segmentSize
                        chopSource := !chopSource |> Seq.skip (Seq.length segment)
                        yield segment
        }

As I said, I think segment, while it gets the job done, is far from good F#; how efficient and idiomatic is chop? What could have been done better?

As for naming, is chop or segment the better name for a function like this, or would something else be more fitting? Also, the argument name segmentSize seems a bit off for F#; would just size be better, or something else?

Functional-first style tends to prefer immutability over mutability, so the fact that you're using ref can sometimes signify that the code could be more idiomatic.

Recursive Solution

But how do you keep track of the state of your program without mutation? The traditional functional solution is to use recursion so that each step of the algorithm is isolated in its own call context. Here is a fairly direct translation of your second example into a recursive function:

let rec chop segmentSize source =
    seq {
            if Seq.isEmpty source then () else
            let segment = source |> Seq.truncate segmentSize
            let rest = source |> Seq.skip (Seq.length segment)
            yield segment
            yield! chop segmentSize rest
    }

Don't be intimidated by my if ... then () else pattern - it's a trick I adopted to avoid needing to indent the rest of function when a precondition is used (I've seen it elsewhere, and it is nice since there's no early return from functions in F#).

Note how I'm using yield! to flatten the sequence from the recursive call into the outer sequence. Also notice that there is no chopSource in this version, because what was chopSource in your example is simply source in the recursive call.

One possible problem with the above solution is that it is not (as far as I know) tail-recursive. I'm not 100% sure how seq works behind the scenes, but I'm assuming that it is not in this case. This may not matter for your problem, but it's still something to consider. It could always be rewritten to use an accumulator argument so that it is tail-recursive and won't overflow the stack.

Another option is to ditch the sequence expression altogether (note that this is still not tail-recursive):

let rec chop segmentSize source =
    if Seq.isEmpty source then Seq.empty else
    let segment = source |> Seq.truncate segmentSize
    let rest = source |> Seq.skip (Seq.length segment)
    Seq.append [segment] (chop segmentSize rest)

Seq.unfold Solution

Although it's nice in this particular case, I'm not a huge fan of recursion because it can sometimes be hard to read and easy to get wrong. I normally use sequence functions as an alternative, as they handle those types of details behind the scenes, and I can worry about the problem at hand. Fortunately, the Seq module provides a nice function, unfold, which does exactly what we are trying to do here - it produces a sequence of values from an evolving state!

The function is called unfold because it is the inverse of fold, which is the equivalent of Enumerable.Aggregate from C#. Here is the documentation:

// Signature:
Seq.unfold : ('State -> 'T * 'State option) -> 'State -> seq<'T>

// Usage:
Seq.unfold generator state

The second argument, state, is the initial value that is going to be used to "seed" the operation (it may seem weird to have this as the second argument, but it is in that order to allow partial application / piping). That initial state value is going to be the first value passed in to our generator function. The generator function optionally returns a tuple containing the current value in the sequence along with the next state value that will be passed back in to generate the next value in the sequence. When the sequence is done,  it returns None.

Enough commentary, here is the implementation:

let chop segmentSize source =
    source
    |> Seq.unfold(fun chopSource ->
        if Seq.isEmpty chopSource then None else
        let segment = chopSource |> Seq.truncate segmentSize
        let rest = chopSource |> Seq.skip (Seq.length segment)
        Some(segment, rest)
    )

So as you can see, we're basically allowing Seq.unfold to call our generator  repetitively as an alternative to recursion. Each iteration returns the segment and the rest of the sequence, which becomes the next state / chopSoure. I'm not actually sure if that is clearer than the recursive version, but it at least won't ever overflow the stack.

As for your naming questions, I think it's somewhat arbitrary. I probably would have called the function segment[ed] (indeed, I think I have). The size argument could go either way. For what it's worth, here's a similar function defined in the Seq module, though it's dealing with a sliding window rather than fixed segments.

Addendum

All these solutions are assuming that it is fine to re-enumerate the sequence, i.e. by truncating once and then skipping. That might be an OK assumption for small, in-memory data-sets, but an IEnumerable doesn't have to allow multiple enumeration, and it can be expensive. As pointed out by Tomas Petricek on that other question, due to the forward-only nature of sequences, whenever you "back up" over a sequence like this it actually has to enumerate the entire thing from the beginning.

But perhaps a sequence isn't the best choice of data types for input into this function. If the data is already in memory, you have a choice of turning it into a list or an array. Arrays are especially nice for this problem due to their support for slicing. Here's a solution that uses arrays instead of sequences:

let segmented (size:int) (source:'t[]) =
    let maxPos = source.Length - 1
    [| for pos in 0 .. size .. maxPos ->
           source.[pos .. min (pos + size - 1) maxPos] |]


BMI calculator/assessor I've just started with Java. The code below calculates Body Mass Index (BMI).
Could someone look for my code and assess it? Maybe I should write something more? All advice will be very helpful.
import java.util.Scanner;

public class BMI {

    public static void main(String[] args) {

        Scanner scanner = new Scanner(System.in);

        double weight = 0;
        double height = 0;
        double bmi = 0;

        System.out.println("Please enter your details to calculate your Body Mass Index.");

        System.out.println("Enter your weight in kilograms:");
        weight = scanner.nextDouble();
        System.out.println("Enter your height in metres:");
        height = scanner.nextDouble();
        bmi = (weight / (height * height));
        System.out.println("Your BMI is: " + bmi);

        if (bmi >= 40) {
            System.out.println("Serious obesity");
        } else if (bmi >= 30) {
            System.out.println("Obesity");
        } else if (bmi >= 25) {
            System.out.println("Overweight");
        } else if (bmi >= 18) {
            System.out.println("Standard");
        } else {
            System.out.println("Underweight");
        }

        scanner.close();

    }

}

Not bad for a beginner, and I agree with Ben Aaronson's comments. Maybe not now, but when you progress, you should change some things:

Break the code up in logical parts (here: methods), and every part should
have one and only one task (Single Responsibility Principle). In your
application, getting the data is one task, calculating is one task and
displaying the results is one task
Error Handling (see Ben's answer)
Testing! Learn to write JUnit tests! It leads "automatically" to better code, not only because you avoid programming errors, but also because you learn to break up code to make it testable
Make your code both scalable, and applicable in different scenarios. For instance, the if-cascade is easy to write and to understand, but it doesn't scale (what if you need 20 cases?), and it is not freely applicable (e.g. you can't pass it around, the ifs are basically "glued" in place)

Although it might be a little bit too advanced for a beginner: Regarding the last point I want to show you a trick even some senior programmers don't know:

import java.util.Scanner;
import java.util.TreeMap;

public class BMI {

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        System.out.println("Please enter your details to calculate your Body Mass Index.");
        System.out.println("Enter your weight in kilograms:");
        double weight = scanner.nextDouble();
        System.out.println("Enter your height in metres:");
        double height = scanner.nextDouble();
        scanner.close();
        double bmi = weight / (height * height);
        System.out.println("Your BMI is: " + bmi);

        TreeMap<Double, String> map = new TreeMap<>();
        map.put(40.0, "Serious obesity");
        map.put(30.0, "Obesity");
        map.put(25.0, "Overweight");
        map.put(18.0, "Standard");
        map.put(0.0, "Underweight");

        System.out.println(map.floorEntry(bmi).getValue());
    }
}

There is a data structure which can do all the if-checks for you, which makes the code trivial to write, scalable, and easy applicable: A TreeMap.

A Map is just a kind of dictionary where you can find "stuff" under a given key. In a TreeMap the keys are ordered. In Java's TreeMap you can not only ask for keys, but also for values between the keys, which get rounded up (ceiling...) or down (floor...) to the next key in the map.


Addition function for two Big Integers represented as std::string This function is from my attempt to write a Big Integer class in C++.

Below, I have a simple addition function.
It takes two strings of arbitrary length which contains only digits (no
'-' sign as well). It returns the sum of the two integers.

Could someone help me optimize for performance?

Since this is my first attempt writing C++ code, I would love for some feedback and improvements in code clarity and style as well. Furthermore, any improvements in security and memory are also appreciated. Please assume that the input will be properly sanitized and no leading zeros are present. I have not included the entire class as it can work as a standalone function.

std::string BigInt::add(const std::string &a, const std::string &b) const
{
    //I think keeping the assignment and declaration of addend in
    // multiple lines prevents copy constructors, right?
    std::string result(a.size() > b.size() ? a:b);
    std::string addend;
    addend = a.size() > b.size() ? b:a;

    char carry = 0;
    char adder = 0;
    for(int cnt = (result.size() - 1); cnt >= 0; cnt--)
    {
        adder = result[cnt] - '0' + carry;
        //cnt is non-negative so it is safe cast
        if( (result.size() - addend.size()) <= static_cast<unsigned>(cnt))
        {
            adder += addend[cnt-(result.size()-addend.size())] - '0';
        }
        result[cnt] = adder%10 + '0';
        carry = adder/10;
    }
    //This propagates the final carry. O(n) insert, can maybe be optimized?
    if(carry)
    {
            result.insert(result.begin(), carry + '0');
    }
    return result;
}

Memory management

There are no leaks, which is great!

However, there is an unnecessary copy being made:

//I think keeping the assignment and declaration of addend in
// multiple lines prevents copy constructors, right?
std::string addend;
addend = a.size() > b.size() ? b:a;

Well, the comment is correct in that the copy constructor doesn't get called, but the copy assignment operator - and that one not only creates a copy of the right hand side operand, but also needs to clean up the original state.

A better way would be to use a reference instead:

const std::string &addend = a.size() > b.size() ? b : a;

Now, no copy is being made, but you can still easily access the smaller operand.

Performance

The simplest way to improve performance would be to chose better data structures/data representations.

The current implementation with std::string has one slight advantage (it is a string, i.e. directly human readable), and some disadvantages:

It wastes a lot of memory (1 byte per decimal digit = 25.6x more memory usage than necessary).
It operates in really small steps (1 byte at a time).
There are some unnecessarily complicated repeated checks.
Inserting a new leading digit (because of carryover) requires all digits to be moved in memory, plus a likely reallocation.

For better performance, these issues should be addressed.

  NOTE: For the best performance, you need to have a look at how the target processor works and understand how you can apply that knowledge to your advantage. That will likely include having to use embedded assembly (e.g. for x86 platforms, having access to the CPU carry flag can be really helpful) and similar tricks.

  But this question is targeting C++, so I'll try to give an example that can be portably done within C++ with not too much overhead.

So, we need to address the issues mentioned above. Let's go through them one at a time:

  Inserting a new leading digit (because of carryover) requires all digits to be moved in memory, plus a likely reallocation.

This can be amended by storing the digits in reverse order (new digit gets inserted at the back instead of the front) and by switching to a std::vector for storage (with maybe some extra capacity to prevent the reallocation).

std::vector<char> BigInt::add(const std::vector<char> &a, const std::vector<char> &b) const
{
    // Some helper variables
    auto min_size = std::min(a.size(), b.size());
    auto max_size = std::max(a.size(), b.size());

    std::vector<char> result(a.size() > b.size() ? a:b);
    const std::vector<char> &addend = a.size() > b.size() ? b : a;

    char carry = 0;
    char adder = 0;

    for(int cnt = 0; cnt < max_size; ++cnt)
    {
        adder = result[cnt] - '0' + carry;

        if(cnt < min_size)
        {
            adder += addend[cnt] - '0';
        }

        result[cnt] = adder%10 + '0';
        carry = adder/10;
    }

    if(carry)
    {
        result.push_back(carry + '0');
    }

    return result;
}

Already looks a lot cleaner, doesn't it?

  There are some unnecessarily complicated repeated checks.

Some of the complicated length calculations already got taken care of, but there is one part that is needlessly repeated:

        if(cnt < min_size)
        {
            adder += addend[cnt] - '0';
        }

Why needlessly? Well, actually we know that this condition will be true for the first min_size iterations of the for loop, and false afterwards. With that knowledge, we can move that check one level higher, and replace the for loop with:

    for(int cnt = 0; cnt < min_size; ++cnt)
    {
        adder = (result[cnt] - '0') + carry + (addend[cnt] - '0');
        result[cnt] = adder%10 + '0';
        carry = adder/10;
    }

    for(int cnt = min_size; carry && cnt < max_size; ++cnt)
    {
        adder = result[cnt] - '0' + carry;
        result[cnt] = adder%10 + '0';
        carry = adder/10;
    }

Of course, now the loop bodies are nearly identical, but that can easily be fixed with some refactoring. Full code:

std::vector<char> BigInt::add(const std::vector<char> &a, const std::vector<char> &b) const
{
    // Some helper variables
    auto min_size = std::min(a.size(), b.size());
    auto max_size = std::max(a.size(), b.size());

    std::vector<char> result(a.size() > b.size() ? a:b);
    const std::vector<char> &addend = a.size() > b.size() ? b : a;

    char carry = 0;

    auto add_digits = [&](char lhs, char rhs)
    {
        auto adder = lhs + rhs + carry;
        carry = adder / 10;
        return adder % 10;
    };

    for(int cnt = 0; cnt < min_size; ++cnt)
    {
        result[cnt] = add_digits(result[cnt] - '0', addend[cnt] - '0') + '0';
    }

    for(int cnt = min_size; carry && cnt < max_size; ++cnt)
    {
        result[cnt] = add_digits(result[cnt] - '0', 0) + '0';
    }

    if(carry)
    {
        result.push_back(carry + '0');
    }

    return result;
}

  It wastes a lot of memory (1 byte per decimal digit = 25.6x more memory usage than necessary).

  It operates in really small steps (1 byte at a time).

Fixing this requires an increase in the digit "size": Before, we were using base-10 digits stored in chars. This can easily be improved to base-256 digits stored in chars (assuming 8 bit char, could be larger!).

But that's basically just calculating on the chars themselves. Most processors can work directly with bigger numbers, like ints (which are usually the largest integer type the target CPU can handle natively). So, why not use ints as "digits"?

  Sadly, ints aren't exactly portable themselves; signed integers can be handled very differently on different processors (e.g. one's complement or two's complement). But: Unsigned intergers are thoroughly specified by the C++ standard, so we can use those.

Also, we can instruct the result vector to preallocate space in case we need space for a new leading digit.

This can easily be added upon the code above for this:

bool check_overflow(unsigned int lhs, unsigned int rhs)
{
    return lhs > std::numeric_limits<unsigned int>::max() - rhs;
}

std::vector<unsigned int> BigInt::add(const std::vector<unsigned int>& a, const std::vector<unsigned int>& b) const
{
    auto min_size = std::min(a.size(), b.size());
    auto max_size = std::max(a.size(), b.size());

    auto result = std::vector<unsigned int>();
    result.reserve(max_size + 1);

    const auto& larger = (a.size() > b.size() ? a : b);
    const auto& smaller = (a.size() > b.size() ? b : a);

    auto carry = 0u;

    auto add_digit = [&](unsigned int lhs, unsigned int rhs)
    {
        auto sum = lhs + rhs + carry;
        carry = (check_overflow(lhs, rhs) || (sum == 0 && carry)) ? 1 : 0;
        return sum;
    };

    for(auto i = 0; i < min_size; ++i)
    {
        result.push_back(add_digit(larger[i], smaller[i]));
    }

    for(auto i = min_size; i < max_size; ++i)
    {
        result.push_back(add_digit(larger[i], 0));
    }

    if(carry) result.push_back(carry);

    return result;
}

  Of course, this isn't perfect performance yet, much can be improved by using knowledge about the target architecture, e.g. by using SSE instructions or similar. Still, this should be quite a lot faster than the original approach, and should be fully portable between different platforms.