Verbose Word/Phrase Reverse in C++ Same exact story as Verbose Word/Phrase Reverse in Node.js, but in C++.

If you don't want to follow the link:

  The assignment is to:

  If input is a word, reverse the word. (e.g. "hello" becomes "olleh")
  If input is a phrase, reverse the order of the phrase but not the actual words. (e.g. "united states of america" becomes "america of states united")

  The other restriction is I am only allowed to use the C++ equivalent of Java String's length, substring, charAt, concatenation and indexOf methods.

Code:

#include <iostream>

std::string ReverseWord(const std::string& word) {
    std::string reverse;
    for (unsigned long i = word.length() - 1; i <= word.length(); --i) reverse += word[i];
    return reverse;
}

std::string ReversePhrase(const std::string& phrase) {
    unsigned long last_space = phrase.length();
    std::string out;
    for (unsigned long i = phrase.length() - 1; i <= phrase.length(); --i) {
        if (phrase[i] == ' ') {
            out += phrase.substr(i + 1, last_space - i - 1) + " ";
            last_space = i;
        }
    }

    out += phrase.substr(0, last_space);
    return out;
}

int pa12() {
    std::string in;
    std::cout << "Enter a word or phrase: ";
    getline(std::cin, in);

    if (std::find(in.begin(), in.end(), ' ') == in.end()) {
        std::cout << "The reversed word is: " << ReverseWord(in) << std::endl;
    } else {
        std::cout << "The reversed phrase is: " << ReversePhrase(in) << std::endl;
    }

    return 0;
}

Prefer to use iterators to iterate through data structures

Look how nice your ReverseWord function becomes when you use the provided rbegin() and rend():

std::string ReverseWord(const std::string& word) {
  return std::string(word.rbegin(), word.rend());
}

Use algorithms when possible

Similarly, your finding of the spaces can be done with a simple string::find() instead.

Apart from that, the code looks reasonable. However...

High level

This is an algorithm that's traditionally implemented as an in-place transformation on a string_view (or array of chars, pre-c++17)

You definitely would want to revisit this and tackle the problem from that angle instead.

It's a bit of a "trick" question, as there is a very simple algorithm to do it, but it's not exactly obvious at first glance. That's why the question asks to reverse a word first. Once you figured out how to reverse a word in-place, the solution to "reverse the words of a phrase" becomes a bit more clear.


jQuery dropdown using each Can someone please review the code and suggest any improvements / changes? Note that this method works but wanted to see if there is a better way for self learning purpose JavaScript/jQuery.
Example
https://jsfiddle.net/tjmcdevitt/Lx0vnb89/4/
var AppType = [
    ['All'],
    ['API'],
    ['AWS'],
    ['Web']
];

    $.each(AppType, function (index, AppType) {
        var option = $('<option></option>').attr("value", AppType).text(AppType);
        $("#ddlApplicationType").append(option);
    });

I have a few suggestions for optimizing your code.
Since ES6 came out it is considered best practice to declare variables as const. It is also a good practice to declare your variables at the top of your script.  For example you are selecting the element #ddlApplicationType everytime you loop over your array. That is not necessary. Just select it once at the top of your script and reuse it as often as you want in your code by referencing the variable const selectApplicationType.  If you know that you are going to reassign a variable you can use the let keyword. Like I did in the $.each function. temp gets reassigned everytime the $.each loops over it. I declared it inside this function because it is a temporarly used variable and will never be needed in the global scope.
When using DOM elements you should check if they exist first. For example if you have multiple pages and a DOM element referenced in your code is missing all the following code will break.
In the example below I used string literals. I think it makes the code a bit more readable. I hope I could help you.
const AppType = [
    ['All'],
    ['API'],
    ['AWS'],
    ['Web']
];
const selectApplicationType = $("#ddlApplicationType")

if (selectApplicationType.length > 0) {
    $.each(AppType, (index, AppType) => {
        let temp = `<option value="${AppType}">${AppType}</option>`
        selectApplicationType.append(temp)
    })
} else {
    console.log(`Element #ddlApplicationType is missing`)
}


Finding added keys, removed keys, and unchanged keys in ordereddict I'm comparing two OrderedDict objects and I need to find added keys, removed keys, and keys that are present in both (the intersection).  Sets are designed for exactly that sort of thing, so my initial attempt involved getting the keys, converting to sets and then comparing/intersecting appropriately.  The trouble I ran into is that I need the order of the original keys to be preserved all the way through the process (hence the use of OrderedDict) and sets don't seem to do that.  I did some googling about ordered sets in python3, but there don't appear to be any native solutions, and I don't really want to bring in another class/library for this one small computation.  Instead, I came up with these lines of code:

def differences(a, b):
    added = []
    removed = []
    overlap = []
    for key in a.keys():
        if key in b:
            overlap.append(key)
        else:
            removed.append(key)

    for key in b.keys():
        if not key in a:
            added.append(key)

    return (added, removed, overlap)

I'm especially interested in hearing suggestions on different ways of tackling this problem, but I'm happy to hear any general suggestions.

A few notes about your current approach:

there is no need to call .keys(), the default iteration over a dict would go over the keys:

for key in a:
    # ...

for key in b:
    # ...

I think doing if key not in a: is a bit more readable than if not key in a:
added can be defined using a list comprehension:

added = [key for key in b if key not in a]

we can define the initial list values using unpacking:

added, removed, overlap = [], [], []

you can remove parenthesis around the return tuple value:

return added, removed, overlap

adding a docstring would really make sense here especially considering the return type and format which can be explicitly mentioned there
we can even go a bit further and return a collections.namedtuple() instead of a regular tuple:

from collections import namedtuple

Difference = namedtuple('Difference', ['added', 'removed', 'overlap'])

and we can add type hints as well

At the end we can get something along these lines:

from collections import namedtuple
from typing import MutableMapping, NamedTuple

Difference = namedtuple('Difference', ['added', 'removed', 'overlap'])

def differences(a: MutableMapping, b: MutableMapping) -> NamedTuple:
    """
    Calculates the difference between two OrderedDicts.

    :param a: OrderedDict
    :param b: OrderedDict
    :return: Difference
    """
    removed, overlap = [], []

    for key in a:
        if key in b:
            overlap.append(key)
        else:
            removed.append(key)

    added = [key for key in b if key not in a]

    return Difference(added=added, removed=removed, overlap=overlap)

Or, if we gonna apply Mathias's suggestion to loop over the a twice we can shorten that to:

from collections import namedtuple
from typing import MutableMapping, NamedTuple

Difference = namedtuple('Difference', ['added', 'removed', 'overlap'])

def differences(a: MutableMapping, b: MutableMapping) -> NamedTuple:
    """
    Calculates the difference between two OrderedDicts.

    :param a: OrderedDict
    :param b: OrderedDict
    :return: Difference
    """

    return Difference(added=[key for key in b if key not in a],
                      removed=[key for key in a if key not in b],
                      overlap=[key for key in a if key in b])


Creating a menu as nested unordered lists from JSON data I have a JSON input using which I have written below function to recursively create an unordered list.

Is there more precise way to achieve it?

$(document).ready(function () {
        var htmlTxt = "";
        var cat = [];
        var htmlStrTxt = "";
        var url = "#";

        function recurseMenu(parent, level) {
            htmlStrTxt = '
';
            for (var x in menuItems) {
                if (menuItems[x].parentId == parent) {
                    htmlStrTxt += '
' + menuItems[x].title + '';

                    if (menuItems[x].childCount > 0) {
                        htmlStrTxt += recurseMenu(menuItems[x].id, level + 1);
                    }
                    htmlStrTxt += '
';
                }
            }
            return htmlStrTxt + '
';
        }

        var htmlTxt = recurseMenu(null, 0);
        $("#listContainer").html(htmlTxt);
     });

At the very least, you should clean up your variables:

htmlTxt is declared as var twice in the same scope.  I don't think you even need it at all.
cat is never used.
htmlStrTxt would be better as a local function within recurseMenu().  A shorter variable name would do just as well.
The level parameter is useless.

$(document).ready(function () {
        var url = "#";

        function recurseMenu(parent) {
            var s = '
';
            for (var x in menuItems) {
                if (menuItems[x].parentId == parent) {
                    s += '
' + menuItems[x].title + '';

                    if (menuItems[x].childCount > 0) {
                        s += recurseMenu(menuItems[x].id);
                    }
                    s += '
';
                }
            }
            return s + '
';
        }

        $("#listContainer").html(recurseMenu());
     });


Converting a text file to a CSV file I'm attempting to learn more about Java and have created a method that takes a text file with stdout (space separated file) and converts it to a CSV file.
I was attempting to use standard Java SE version 8.

Is there a better, more efficient, way of doing this?

The logic is:

Open file
Read file by line into string so it can be split
Split string removing spaces, back into array
Join with StringJoiner using ,
Convert back to string to remove leading ,
Update final array to be returned

Method to open file:

public void OpenFile(String fileName)
{
    try
    {
        subFile = new Scanner (new File(fileName));
    }
    catch (Exception e)
    {
        System.out.println("File dosn't exist.");
    }
}

Method to convert:

    public String[] TextToCsvArray(String[] fileArray)
{
    int i=0;
    while(subFile.hasNext())
    {
        String line = subFile.nextLine();
        String[] split = line.split("\\s+");
        StringJoiner joiner = new StringJoiner(",");

        for (String strVal: split)
            joiner.add(strVal);

        line = joiner.toString();
        line = line.startsWith(",") ? line.substring(1) : line;
        fileArray[i++] = line;
    }
    return fileArray;
}

If your input is already separated correctly by spaces, it seems all you need to do is to convert those into commas and you're good. I'm not sure you need to go to/from arrays.

I would replace the code inside the loop:

{
    String line = subFile.nextLine();
    String[] split = line.split("\\s+");
    StringJoiner joiner = new StringJoiner(",");

    for (String strVal: split)
        joiner.add(strVal);

    line = joiner.toString();
    line = line.startsWith(",") ? line.substring(1) : line;
    fileArray[i++] = line;
}

with this:

{
    String line = subFile.nextLine();
    line.trim().replaceAll(" +", " "); //check for double spaces
    line.replace(' ', ',');   //replace space with comma
    fileArray[i++] = line;
}

Oh, and check for the array size like someone else mentioned.


Calculating the area of various 2D shapes I've made a simple program that can calculate the area of various 2D shapes. I'd like your expereanced eyes to asses my code. Be as harsh as you want but please be specific and offer me ways in which I can improve my coding.

shape = ("triangle","square","rectangle","circle")                  # This is a tuple containing the string names of all the supported shapes so far.
constants = (3.14)                                                  # An object containing the value of pi. I will add more constants in future and turn this into a tuple.
start_up = input("Do you want to calculate a shape's area?: ")      # Fairly self explainatory

if start_up != "yes" or "no":                                       # This is to prevent the program failing if the user answers the start_up question
   print("Sorry but I don't understand what your saying")           # with an answer outside of the simple yes/no that it requires
   start_up = input("Do you want to calculate a shape's area?: ")   # re-iterates the start up question.

   while start_up == "yes":                                            # This is the condition that obviously contains the bulk of the program.
      target_shape = input("Choose your shape: ")                       # Once again, self explainatory.

      if target_shape == shape[0]:                                      # Uses the shape tuple to bring forth the case for the first shape, the triangle.
         h = float(input("give the height: "))                          # These Two lines allow the user to input the required values to calculate the area of the triangle.
         b = float(input("give the base length: "))                     # They are using the float funtion to increase the accuracy of the answer.
         area_triangle = h * 1/2 * b                                    # The formula we all learned in school.
         print("the area of the %s is %.2f" % (shape[0],area_triangle)) # This generates a format string which uses both the shape tuple and the area_triangle object
                                                                # to display the answer. note the %.2f operand used to specify how many decimal places for the answer.

      if target_shape == shape[1]:                                      # Square
         l = float(input("give the length: "))
         area_square = l ** 2
         print("the area of the %s is %.2f" % (shape[1],area_square))

      if target_shape == shape[2]:                                     # Rectangle
         l = float(input("give the length: "))
         w = float(input("give the width: "))
         area_rectangle = l * w
         print("the area of the %s is %.2f" % (shape[2],area_rectangle))

      if target_shape == shape[3]:                                     # Circle
         r = float(input("give the radius: "))
         pi = constants                                                # In this line we are calling forth the constants object to use the value of pi.
         area_circle = 2 * pi * r
         print("the area of the %s is %.2f" %(shape[3],area_circle))

         start_up = input("Do you want to calculate another shape's area?: ") # This line allows the user the chance to do just what it says in the brackets.

      else:                                                                   # This is to safegaurd against failure in the event of the user
        print("That shape's not in my database")                             # inputing a shape non-existant in the shape tuple.
        start_up = input("Do you want to calculate another shape's area?: ")

        if start_up == "no":                                                      #This is for when the user no longer wants to use the program.
           print("Goodbye then!")
           input("Press any key to close")

Since you're not using shape extensively I would get rid of that data structure all together. It would be easier to read inline then having to look to the margin for comments.

For example:

  if target_shape == 'square':
     l = float(input("give the length: "))
     area_square = l ** 2
     print("the area of the square is %.2f" % (area_square))

When comparing strings if you don't want to be strict you should normalize them before compare.

For example:

'Square'.lower() == 'square'

so your code would become:

  if target_shape.lower() == 'square':
     l = float(input("give the length: "))
     area_square = l ** 2
     print("the area of the square is %.2f" % (area_square))

You're not accounting for any errors. What if the user hits enter without giving a length?

ValueError: could not convert string to float


Timed series of events. Logic for organizing events based on time? I would like to execute a series of events at the time at which they occur.

The events are stored in a list and always sorted by time of execution.

volatileEvents.Sort(SortScriptEventAscending.Comparer);

I would like to have my update method execute the following points.

The list should be checked to see if it contains any events (if not then don't continue)
The events should be executed and remain in the list until the next event is executed
Once the current event is executed the previous event should be removed
Events should always be checked to see if they have either expired or are yet to occur

I have the basic conditions implemented but the algorithm doesn't work as intended.

        float currentTime = game.ScriptTimer.CurrentTime;

        int numEvents = volatileEvents.Count;

        // Check for no events
        if (numEvents == 0)
        {
            return;
        }

        // Check for expired events
        while (volatileEvents[0].Time < currentTime)
        {
            volatileEvents.RemoveAt(0);
        }

        // Check if the current event has yet to occur
        if (volatileEvents[0].Time > currentTime)
        {
            return;
        }

        // Protect against index range error
        int next = numEvents == 1 ? 0 : 1;

        ScriptEvent currentEvent = volatileEvents[0];
        ScriptEvent nextEvent = volatileEvents[next];

        // Update the current value based on the script and event parameters
        float value = currentEvent.Interpolate ?
            MathTools.Lerp2D(currentEvent.Value, nextEvent.Value, currentEvent.Time, currentTime, nextEvent.Time) :
            currentEvent.Value;

I would like some help to achieve the four bullet points as the basic conditions are there but the method's order of execution is incorrect.

My simplified answer (without interpolation or looping):

        int numEvents = events.Count;

        // Check if there are any events to execute
        if (eventIndex == numEvents)
        {
            return;
        }

        // Check if the current event has yet to occur
        if (events[eventIndex].Time > currentTime)
        {
            return;
        }
        else
        {
            // Update the property based on the current event
            UpdateProperty(events[eventIndex].Value);

            // Current event has been executed so increase the event index
            eventIndex++;
        }


Haskell Bencoded Dictionary Parser I have been working on a Bencoded string parser in order to improve my knowledge
of Haskell. After running the code through hlint, I still have a few questions:

As I noted in the comments, the key value in the definition of BMapT should
always be a Bstr. Is there any way to enforce a particular data constructor when
pattern matching an algrebraic datatype?
Since Bencoded dictionaries are generally stored as human readable text, I
made an instance of the Show typeclass so that I could take Bencode structures
and turn them into text. I was wondering if this is the right thing to do, or if
Haskell has another typeclass for storage/serialization a la Python's repr.
This question is kind of nebulous, but would you consider this code
'idiomatic'? Code without style isn't worth writing :)

Code:

-- This is an implementation of Bencoding for bittorrent as described at
-- http://www.bittorrent.org/beps/bep_0003.html

module Bencode where

import Text.Parsec.ByteString
import Text.Parsec.Char
import Text.Parsec.Prim
import Text.Parsec.Combinator
import qualified Text.Parsec.Error as PE
import Data.Char
import qualified Data.Map as M
import qualified Control.Monad as Mon

-- Technically this first argument can only be a Bstr, but I don't know
-- how to express that
type BMapT = M.Map Bencode Bencode

-- I have no idea why Bstr is okay as a String when it's a ByteString
data Bencode =  Bint Integer
              | Bstr String
              | Blist [Bencode]
              | Bmap BMapT
              deriving (Eq, Ord)

instance Show Bencode where
    show (Bint i) = "i" ++ show i ++ "e"
    show (Bstr s) = (show . length) s ++ ":" ++ s
    show (Blist bs) = 'l':concatMap show bs ++ "e"
    show (Bmap bm) = M.foldlWithKey (\a k b -> a ++ show k ++ show b) "d" bm  ++ "e"

-- Parse a Bencoded Integer
bInt :: Parser Bencode
bInt = do char 'i'
          num <- validNum
          char 'e'
          return $ Bint num
       -- This parser parses valid integers in Bencodings
       where validNum = do neg <- option ' ' (char '-')
                           d <- digit
                           case digitToInt d of
                                -- Only time the first digit == 0 is "i0e"
                                0 -> if neg == ' ' then
                                        -- "i0e" allowed but NOT "i-0e" or zero padded integer
                                        lookAhead (char 'e') >> return 0
                                     else
                                        parserFail "Can't have a negative zero"
                                _ -> many digit >>= \xs -> return $ read (neg:d:xs)

-- Parse a Bencoded String
bString :: Parser Bencode
bString = do ss <- many1 digit
             char ':'
             let size = read ss
             Mon.liftM Bstr $ count size anyChar

bList :: Parser Bencode
bList = do char 'l'
           ls <- many (bInt <|> bString <|> bList <|> bMap)
           char 'e'
           return $ Blist ls

-- A parser which parses bencoded dictionaries
bMap :: Parser Bencode
bMap = do char 'd'
          entries <- many dictEntry
          char 'e'
          return $ Bmap $ M.fromList entries

-- This parser will parse a key-value pair
dictEntry :: Parser (Bencode, Bencode)
dictEntry = do key <- bString
               value <- bString <|> bList <|> bInt <|> bMap
               return (key, value)

-- This function reads a torrent file. readBencodedFile "filename" reads
-- that filename and returns the parsed bencoded dictionary
readBencodedFile :: String -> IO (Either PE.ParseError Bencode)
readBencodedFile = parseFromFile bMap

Below is your code with some minor tweaks and the comments moved into standard Haddock form.

module Bencode (readBencodedFile, BencodedValue(..), bencode, bencodeBS) where

import qualified Control.Monad as Mon
import Data.ByteString (ByteString)
import Data.ByteString.Char8 (pack)
import qualified Data.Map as M
import Text.Parsec.ByteString as ParseBS
import Text.Parsec.Char (anyChar, char, digit)
import Text.Parsec.Prim ((<|>), parserFail, runParser)
import Text.Parsec.Combinator (count, many1, option)
import qualified Text.Parsec.Error as PE

type BMapT = M.Map BencodedValue BencodedValue

data BencodedValue = Bint Integer
                   | Bstr String
                   | Blist [BencodedValue]
                   | Bmap BMapT
                   deriving (Eq, Ord, Show)

-- |Transform a 'BencodedValue' back to an encoded string
bencode :: BencodedValue -> String
bencode (Bint i)   = "i" ++ show i ++ "e"
bencode (Bstr s)   = (show . length) s ++ ":" ++ s
bencode (Blist bs) = 'l':concatMap bencode bs ++ "e"
bencode (Bmap bm)  = M.foldlWithKey (\a k b -> a ++ bencode k ++ bencode b) "d" bm  ++ "e"

-- |Return a 'BencodedValue' value to it original encoded 'ByteString' form.
bencodeBS :: BencodedValue -> ByteString
bencodeBS = pack . bencode

{-|
  'readBencodedFile' reads data from a torrent file and returns the parsed
  result as a dictionary in a 'BencodedValue'.
  The 'String' is the filename of the torrent to read.
 -}
readBencodedFile :: String -> IO (Either PE.ParseError BencodedValue)
readBencodedFile = parseFromFile bMap

------------------------
-- Internal functions --
------------------------

-- |Parse a Bencoded Integer
bInt :: ParseBS.Parser BencodedValue
bInt = do char 'i'
          neg  <- option ' ' (char '-')
          nums <- many1 digit
          char 'e'
          mkBint neg nums
  where
    mkBint ' ' ['0']    = return $ Bint 0
    mkBint '-' ('0':ns) = parserFail "Can't have a negative zero"
    mkBint _   ('0':ns) = parserFail "Invalid number with a leading zero"
    mkBint '-' nums     = return $ Bint $ read ('-':nums)
    mkBint _   nums     = return $ Bint $ read nums

-- |Parse a Bencoded String
bString :: ParseBS.Parser BencodedValue
bString = do nums <- many1 digit
             char ':'
             mkBstring (read nums)
  where
    mkBstring size = do cs <- count size anyChar
                        return $ Bstr cs

-- |Parse a Bencoded List
bList :: ParseBS.Parser BencodedValue
bList = do char 'l'
           ls <- many1 (bInt <|> bString <|> bList <|> bMap)
           char 'e'
           mkBlist ls
  where
    mkBlist = return . Blist

-- |Parse a Bencoded dictionary
bMap :: ParseBS.Parser BencodedValue
bMap = do char 'd'
          entries <- many1 dictEntry
          char 'e'
          mkBmap entries
  where
    mkBmap = return . Bmap . M.fromList

-- |Parse a Bencoded dictionary entry.
-- This is an internal function declared outside of 'bMap' to
-- facilitate testing.
dictEntry :: ParseBS.Parser (BencodedValue, BencodedValue)
dictEntry = do key <- bString
               value <- bString <|> bList <|> bInt <|> bMap
               return (key, value)

Answers to your questions.

Without getting REAL fancy there is no good way limit the BMap argument to a Bstr while it is an member of algebraic type.
Your analogy to repr from Python is a good one. Typically if there is a Haskell Show instance there is also a Read instance so show . read . show is valid. In your case the result of show needs to be parsed again and you do not expose a parse from string/bytestring. I added a bencode and bencodeBS which takes a BencodedValue and outputs a String or ByteString.
Look at where I simplified or renamed your items to see suggestions on idiomatic usage. On the whole you did pretty well. If I had more time I would have liked to transform your parser functions into Applicative style. By transitioning to the mkFoo functions I started that process for you if you like.
In the code you ask about why this is a String when the ByteString is the input. It is because the parsers are returning Strings parsed from the ByteString. If the performance really matters you could transition over to Attoparsec.


Finding the largest absolute difference in a list I'm moving away from map, reduce and filter in favour of list comprehensions. I like list comps but I feel like this is getting on the unreadable side.

Is this the most idiomatic way of achieving this? Is it easy enough to read?

int_list = [-1,-3,5,7]

absolute_diffs = [max( [abs( int_2 ) - abs( int_1 ) for int_2 in int_list] ) for int_1 in int_list]

print max( absolute_diffs )

A more idiomatic and readable solution would be to use itertools.combinations(list, 2) to take all pairs.

max(abs(a) - abs(b) for a, b in itertools.combinations(int_list, 2))

However, that is O(n2). A smarter O(n) solution would be to subtract the minimum absolute value from the maximum absolute value.

absolutes = [abs(elem) for elem in int_list]
print max(absolutes) - min(absolutes)


Finding indices of CSV headers in Java I'm working on a project where I needed to extract the indices for the headers of a CSV file to initialise objects. I thought that I should work on my documentation process and learn how to document code, seeing as I'm always reading about people not documenting code.

/**
 * Returns the indices of searched for headers, e.g. if the file looks like this: (Name Email
 * Phone Age) and we call the method: findIndices("Name", "Age", "Email", "Phone"), if returns
 * [0,3,1,2].
 *
 * Ignores case of letters, i.e. Age and AGE "is" the same.
 *
 * @param input Strings to search for in the header
 * @return int[] with indices for the columns
*/
public int[] findIndices(String... input) throws IllegalArgumentException {
    int[] indices = new int[input.length];
    if (input.length > this.headers.length) {
      throw new IllegalArgumentException("Amount of searched for headers, " + input.length
      + ", exceeded the actual amount of headers: " + this.headers.length);
    }
    boolean found = true; // Assume we will find all headers.
    int k = 0; // Counter variable.
    for (int i = 0; i < this.headers.length; i++) {
        for (String s : input) {
            if (s.equalsIgnoreCase(this.headers[i])) {
                indices[k++] = i;
                found = true;
            } else {
                found = false;
            }
        }
    }
    if (!found) {
        throw new IllegalArgumentException(
            "One or more of input arguments could not be found in the headers.");
    }
    return indices;
}

My own thoughts:

The found flag seems poorly handled.
This is part of a custom CSV-reader class, with a headers private field. This seems a bit weird to me, this could maybe be static and take a String[] as input as well.
Should this actually throw an exception or should it just return a "bad" index? But I'm using an array to represent it. Null, maybe?

Edit: Found a bug which did not show up in my unit tests due to a misunderstanding of the JUnit api, found -> !found.

Since you mentioned unit tests, here are 2 unit tests that proves your code does not work (Based on name, age, email, phone header):

@Test
public void testFindIndicesThrowsException()
{
    ClassName x = new ClassName();

    try
    {
        int[] result = x.findIndices("Name", "age", "FieldThatDoesNotExist", "Phone");
        fail("expected exception was not thrown!");
    }
    catch (IllegalArgumentException exception)
    {
        assertEquals("One or more of input arguments could not be found in the headers.", exception.getMessage());
    }
}

@Test
public void testFindIndices()
{
    ClassName x = new ClassName();

    int[] result = x.findIndices("Email", "Phone", "Name", "age");
    assertEquals(4, result.length);
    assertEquals(2, result[0]);
    assertEquals(3, result[1]);
    assertEquals(0, result[2]);
    assertEquals(1, result[3]);
}

Explanation: You are setting a boolean in a loop. This means it could be set to false on one iteration, then set to true on the next.

Instead, you can set the index to the one found, and if it's not found, throw an error:

int k = 0;

for (String s : input)
{
    indices[k] = -1;

    for (int i = 0; i < this.headers.length; i++)
    {
        if (s.equalsIgnoreCase(this.headers[i]))
        {
            indices[k] = i;
        }
    }

    // If we never found the item
    if (indices[k] == -1)
    {
        throw new IllegalArgumentException(
            "One or more of input arguments could not be found in the headers.");
    }

    k++;
}


Max element of input iterator range I came across this SO question on why std::max_element requires a ForwardIterator (as apposed to an InputIterator). Having a max_element-like algorithm for InputIterator would be useful, as it could then be used with objects like std::istream_iterator or boost::transform_iterator.

I came up with the following algorithm for this:

#include <iterator>   // std::iterator_traits, std::advance
#include <stdexcept>  // std::logic_error
#include <algorithm>  // std::for_each
#include <functional> // std::less
#include <utility>    // std::move

template <typename InputIterator, typename Compare>
typename std::iterator_traits<InputIterator>::value_type
max_input(InputIterator first, InputIterator last, Compare comp)
{
    if (first == last) throw std::logic_error {"max_input given empty range"};

    using ValueType = typename std::iterator_traits<InputIterator>::value_type;

    ValueType result {*first};

    std::advance(first, 1);

    std::for_each(first, last, [&result, comp] (ValueType curr) {
        if (comp(result, curr)) result = std::move(curr);
    });

    return result;
}

Example:

#include <sstream>
#include <iostream>

int main()
{
    std::istringstream str("0.1 0.2 0.3 0.4 0.2 0.0 0.1");

    const auto max = max_input(std::istream_iterator<double>(str), std::istream_iterator<double>());

    std::cout << "max is " << max << std::endl;

    return 0;
}

I'm looking for general feedback/improvements, especially on the decision to throw for an empty range. I was also deliberating whether the algorithm should also return the index of the maximum (e.g. in a std::pair).

Don't use a lambda

This code:

std::for_each(first, last, [&result, comp] (ValueType curr) {
    if (comp(result, curr)) result = std::move(curr);
});

is just a lot messier than the non-lambda equivalent:

for (; first != last; ++first) {
    ValueType cur = *first;
    if (comp(result, cur)) {
        result = std::move(cur);
    }
}

I don't see the advantage in the lambda.

Avoid the copy

Once you advance an input iterator, all earlier iterators are invalidated. But you can dereference the same one multiple times. In this case, you're incurring an unnecessary copy of ValueType, you can just write:

for (; first != last; ++first) {
    if (comp(result, *first)) {
        result = std::move(*first);
    }
}

Move iterators

Dereference likely returns a const T& - so you probably want to wrap the iterators in std::make_move_iterator first. That will actually let you move-from the iterator, instead of looking like you're moving from and actually copying from.

advance is excessive

You can just write:

ValueType result{*first++};

Or:

ValueType result{*first};
++first;

There's no need for advance() - you know operator++ is supported.

Throwing on empty range

Honestly, I'd prefer it just be undefined behavior and avoid the first check. Let the user deal with it. That's just a personal preference though.

Additionally, you could return an optional<ValueType>, returning none if the range is empty.

Defer to std::max_element?

Since std::max_element is more efficient than yours for forward iterators, it'd be nice to defer to it if you're given a forward iterator:

template <class It, class Cmp>
typename std::iterator_traits<It>::value_type
max_input(It first, It last, Cmp mp) {
    // possibly check empty range and throw here, if that's what you want
    return max_input(first, last, cmp, typename std::iterator_traits<It>::iterator_category{});
}

template <class It, class Cmp>
typename std::iterator_traits<It>::value_type
max_input(It first, It last, Cmp cmp, std::input_iterator_tag) {
    /* your implementation */
}

template <class It, class Cmp>
typename std::iterator_traits<It>::value_type
max_input(It first, It last, Cmp cmp, std::forward_iterator_tag) {
    return *std::max_element(it, last, cmp);
}

Excessive commenting

Avoid this sort of commenting style:

#include <iterator>   // std::iterator_traits, std::advance
#include <stdexcept>  // std::logic_error
#include <algorithm>  // std::for_each
#include <functional> // std::less
#include <utility>    // std::move

It's just verbose and provides no value.


to use a constructor or not use a constructor I have a validator class, whose purpose is to validate input data.  Initially I wrote the class like this:

export class ValidatorForInputs {

  constructor(inputData) {
    this._data = inputData;
  }

  validate() {
    let results = {};

    let validateInputDataForMissingKeys = checkInputDataForMissingKeys(this._data);
    if (validateInputDataForMissingKeys) results = { ...results, ...validateInputDataForMissingKeys };

    let validateInputDataForMissingValues = checkInputDataForMissingValues(this._data);
    if (validateInputDataForMissingValues) results = { ...results, validateInputDataForMissingValues };

    let validateInputDataMaximums = checkInputDataMaximums(this._data);
    if (validateInputDataMaximums) results = { ...results, validateInputDataMaximums };

    return results;

  }

}

to be called like this:

  let validatorForInputs = new ValidatorForInputs(inputData);
  let validationForInputsResult = validatorForInputs.validate();
// do something with validationForInputsResult

But then I wondered, is the constructor really necessary when the data could be passed directly to the validate method like so:

  export class ValidatorForInputs {

    validate(data) {
      let results = {};

      let validateInputDataForMissingKeys = checkInputDataForMissingKeys(data);
      if (validateInputDataForMissingKeys) results = { ...results, ...validateInputDataForMissingKeys };

      let validateInputDataForMissingValues = checkInputDataForMissingValues(data);
      if (validateInputDataForMissingValues) results = { ...results, validateInputDataForMissingValues };

      let validateInputDataMaximums = checkInputDataMaximums(data);
      if (validateInputDataMaximums) results = { ...results, validateInputDataMaximums };

      return results;

    }

  }

and then called like this:

  let validatorForInputs = new ValidatorForInputs();
  let validationForInputsResult = validatorForInputs.validate(inputData);
// do something with validationForInputsResult

or even called in a one liner like this:

let validationForInputsResult = ( new ValidatorForInputs() ).validate(inputData);
// do something with validationForInputsResult

With a little bit of testing, I found that the code behaves the same way.  I read the what the docs have to say about constructors but did not find any information that answers this question.

So, in a case like this, should a constructor be used or not?  If so, why?  If not, why not?

I think a class provides benefits when you need two things:

Persistent data associated with an instance, and
Methods that do something with the data

If you only need to consolidate data in a single data structure, an object or array is simpler and more suitable.

If you only need to perform an operation on data and produce a result, and then have other not-very-related code work with the result, a plain function is simpler and more suitable.

Your situation looks to fall into the second category. You have data that you want to validate, but all you want to do is validate, and the possible class wouldn't be doing anything else. If all you're doing with a class to set a property, then retrieve that property in a method to process data, and then you never use the instance again, there's not much point to having the class at all - it's just noise. I'd use a standalone function instead.

export const validate = (data) => {
  const results = {};

  const validateInputDataForMissingKeys = checkInputDataForMissingKeys(data);
  if (validateInputDataForMissingKeys) Object.assign(results, validateInputDataForMissingKeys);

  const validateInputDataForMissingValues = checkInputDataForMissingValues(data);
  if (validateInputDataForMissingValues) Object.assign(results, { validateInputDataForMissingValues });

  const validateInputDataMaximums = checkInputDataMaximums(data);
  if (validateInputDataMaximums) Object.assign(results, { validateInputDataMaximums });

  return results;
};

Then call it like:

const validationForInputsResult = validate(inputData);

Also note that you should strongly prefer using const instead of let. let is a warning to other readers of your code (possibly including yourself, in the future) that you may be reassigning the variable in the future. If you're not going to reassign, better to use const, since it requires less cognitive overhead.

The checkInput functions and validateInput variables look a bit repetitive. If possible, it would be nice to change things so that they return an empty object instead of a falsey value, and have checkInputDataForMissingValues and checkInputDataMaximums return objects with validateInputDataForMissingValues and validateInputDataMaximums properties. That way, you can spread them directly into the results object:

const validate = data => ({
  ...checkInputDataForMissingKeys(data),
  ...checkInputDataForMissingValues(data),
  ...checkInputDataMaximums(data),
});

Having 3 separate checkInputData functions plus validate feels a tiny bit off to me. Organization-wise, you could consider putting them all into an object, together with validate if they're not used anywhere else, that way all the related functionality is in a single data structure:

export const validator = {
  checkInputDataForMissingKeys(data) {
    // ...
  },
  checkInputDataForMissingValues(data) {
    // ...
  },
  checkInputDataMaximums(data) {
    // ...
  },
  validate(data) {
    return {
      ...this.checkInputDataForMissingKeys(data),
      ...this.checkInputDataForMissingValues(data),
      ...this.checkInputDataMaximums(data),
    };
  }
};

and

const validationForInputsResult = validator.validate(inputData);


Mini JavaScript thread pool with maximum number of concurrently running tasks I wrote this mini thread pool in JavaScript that takes a maximum number of concurrently running tasks and runs them in the order it was given. I appreciate any feedback or review. Thank you.
class TaskPool {
  constructor(poolSize) {
    this.poolSize = poolSize;
    this.queue = [];
    this.runningTasks = 0;
    this.scheduled = null;
    this.id = 0;
  }

  backgroundRunner() {
    if (this.queue.length) {
      if (this.runningTasks < this.poolSize) {
        const index = 0;
        const [[task, id]] = this.queue.splice(index, 1);
        this.runningTasks++;
        task(id).finally(() => {
          this.runningTasks--;
          this.backgroundRunner();
        });
      } else {
        console.log('queue is full');
        if (!this.scheduled) {
          console.log('scheduled for later');
          this.scheduled = setTimeout(() => {
            this.backgroundRunner();
            this.scheduled = false;
          }, 1000);
        }
      }
    }
  }

  run(task) {
    this.queue.push([task, ++this.id]);
    this.backgroundRunner();
  }
}

Tests:
function task(id) {
  //create an ID
  return new Promise((resolve, _) => {
    console.log('running task #', id);
    setTimeout(() => {
      console.log('done running task #', id);
      resolve();
    }, 400);
  });
}

const pool = new TaskPool(3);
for (let i = 0; i < 10; i++) {
  pool.run(task);
}

jsfiddle

Some suggestions:

It's not entirely clear until getting to the bottom of the class what the queue array holds. Maybe call it something more precise like unstartedTasks or unstartedTasksQueue.

You can return early to avoid nested indentation - cutting down on unnecessary indentation can make it clearer exactly which block a } at the end corresponds to. For example, backgroundRunner could be made to be:
backgroundRunner() {
  if (!this.queue.length) {
    return;
  }
  // lots of code

The scheduled for later section looks superfluous since unstarted tasks will be run immediately upon the completion of a task, due to the
task(id).finally(() => {
  this.runningTasks--;
  this.backgroundRunner();
});

When you want to remove the first element from an array and use it, use shift, not splice. This:
const [[task, id]] = this.queue.splice(index, 1);

can be
const [task, id] = this.queue.shift();

Unless the insertion index of the task needs to be saved, you could simplify the queue by incrementing and using the id not when the task is pushed, but when the task gets executed. This way the queue only needs to be an array of functions, not an array of [task, id]s.

See below for the version I'd prefer, if a class-based implementation is required. I use # to clearly distinguish what's exclusively private methods and properties from a public method:

class TaskPool {
  #unstartedTasksQueue = [];
  #runningTaskCount = 0;
  #id = 0;
  #runningTaskLimit;
  constructor(runningTaskLimit) {
    this.#runningTaskLimit = runningTaskLimit;
  }
  insertTask(fn) {
    if (this.#runningTaskCount === this.#runningTaskLimit) {
      this.#unstartedTasksQueue.push(task);
    } else {
      this.#startTask(fn);
    }
  }
  #startTask(fn) {
    this.#runningTaskCount++;
    fn(this.#id++).finally(() => {
      if (this.#unstartedTasksQueue.length) {
        this.#startTask(this.#unstartedTasksQueue.shift());
      } else {
        this.#runningTaskCount--;
      }
    });
  }
}

function task(id) {
  //create an ID
  return new Promise((resolve) => {
    console.log('running task #', id);
    setTimeout(() => {
      console.log('done running task #', id);
      resolve();
    }, 4000);
  });
}

const pool = new TaskPool(3);
for (let i = 0; i < 5; i++) {
  pool.insertTask(task);
}

But all the private methods and properties, despite making the code's intent a bit clearer, can look a bit ugly. Since the external user of the class is only interfacing with 2 things (the constructor, and the run method), I'd prefer to make a function instead of a class, which returns the run (or, as I've named it, insertTask) method, so as to rely on the closure instead of on #s:

const makeTaskPool = (runningTaskLimit) => {
  const unstartedTasksQueue = [];
  let runningTaskCount = 0;
  let id = 0;
  const startTask = (fn) => {
    runningTaskCount++;
    fn(id++).finally(() => {
      if (unstartedTasksQueue.length) {
        startTask(unstartedTasksQueue.shift());
      } else {
        runningTaskCount--;
      }
    });
  };
  return (fn) => {
    if (runningTaskCount === runningTaskLimit) {
      unstartedTasksQueue.push(task);
    } else {
      startTask(fn);
    }
  };
};

function task(id) {
  //create an ID
  return new Promise((resolve) => {
    console.log('running task #', id);
    setTimeout(() => {
      console.log('done running task #', id);
      resolve();
    }, 4000);
  });
}

const insertTask = makeTaskPool(3);
for (let i = 0; i < 5; i++) {
  insertTask(task);
}


Collision detection accuracy How do you guys think I can improve the code for collision detection on my website asteroidfield.eu5.org? The current code is

var dx = Math.abs(c1.getcx() - c2.getcx());
var dy = Math.abs(c1.getcy() - c2.getcy());
var dist = Math.sqrt((dx * dx) + (dy * dy));
if (dist - (c1.getR() + c2.getR()) <= 0) {
  if (done === false && invinc !== true) {
    game_over = true;
    done = true;
  }
}

C1 refers to the player sprite, and C2 is the enemy sprite. Getcx is essentially collecting the X location of the edge of what is essentially a collision detection circle, and gety does the same, but collects the Y location. GetR returns the radius of the collision detection circle.

One thing you can improve on is the getting of the absolute value. In some browsers, bitwise absolute is faster than Math.abs. But it depends on the browser's implementation.

//bitwise absolute
function abs(n){
  return (n^(n>>31))-(n>>31);
}

You could also factor this part out into a variable for readability. The result of this is a boolean. The explanation will be in the next part:

//foo true if less than or equal to 0, false if greater
var foo = dist - (c1.getR() + c2.getR()) <= 0;

Also, in this bit of code, since done and invinc are boolean, there's no point comparing them to true and false. You can use them directly in the condition.

if (done === false && invinc !== true) //when false, and not true (false)

if (!done && !invinc)                  //when not true (false) and not true (false)

In JS, assignment operations "spill left". You can do the following, assigning true to done and "spill" the same value over to game_over.

game_over = done = true;

Also, due to the structure of the code, the previous only happens when our condition assigned to foo is less than or equal to zero or true. So let's modify the condition to this:

if(foo && !done && !invinc) game_over = done = true;

So in the end, your code will look like this:

var dx = Math.abs(c1.getcx() - c2.getcx());
var dy = Math.abs(c1.getcy() - c2.getcy());
var dist = Math.sqrt((dx * dx) + (dy * dy));
var foo = dist - (c1.getR() + c2.getR()) <= 0;

if (foo && !done && !invinc) game_over = done = true;


Sokoban solution rotator in Perl This is a project I always meant to do, but now that I've done it, I am wondering if/how I can do it better. I've made some fixes and done general testing on the parsing to make sure it works, but I sense there is more to do that could be very instructive to me in terms of organizing my code and thoughts.

This came about because I played several Sokoban games and wrote solutions only to notice that many levels were rotations of others, so I wanted to save time. (There are Sokoban solvers out there, but I wanted to provide explanations with the solutions.)

The code below takes a text file (it need only be one line - the comments provide one) and user-suggested rotations (argument 1 on the command line) and changes all directions(2U, 3L, 4D, 5R) to how the user wants the board rotated.

Some of it seems a bit brute force, though, especially the for loop and the regular expression (I've only recently moved beyond ending with just /g or /gi). In particular, I am wondering whether I should use an array instead, especially since I may want an option to convert the full words "up"/"right"/"left"/"down" to their rotational equivalents.

Any suggestions big or small are appreciated...whether it's technical or code organization.

This seems like the sort of exercise that might help me learn other languages I'm not familiar with, so I wanted to see how things went with a language I was okay at, first.

Thanks to all who are able to contribute, and I hope my example is somewhat interesting.

################################
#sokoban solution rotator
################################
#sok.txt has lines like so: Your way to the end: 2U 4R 3D 5L 2u 4r 3d 5l.
#the command line argument is any combination of l r h u d v (d and v are equivalent to u, vertical flips. L/R rotate left/right, h = horizontal flip)
#

use strict;
use warnings;

my $file = "sok.txt";
my $dir;
my $anyDif; # checks for differences once we've parsed rotations

# this hash transforms a move to a different direction. By default each move points to itself.
my %rotate = ( 'u' => 'u',
  'r' => 'r',
  'd' => 'd',
  'l' => 'l');

if (!defined($ARGV[0]))
{
  die ("You need a command line argument (no spaces) to use rotations: R, L, H and V/U/D rotate right, left, horizontally and vertically.");
}

my @adjust = split(//, lc($ARGV[0]));

for $dir (@adjust)
{
  if ($dir eq 'r') # rotate right
  {
    ($rotate{'r'}, $rotate{'d'}, $rotate{'l'}, $rotate{'u'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  if ($dir eq 'l') # rotate left
  {
    ($rotate{'l'}, $rotate{'u'}, $rotate{'r'}, $rotate{'d'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  if (($dir eq 'v') || ($dir eq 'u') || ($dir eq 'd')) # vertical flip
  {
    ($rotate{'d'}, $rotate{'r'}, $rotate{'u'}, $rotate{'l'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  if ($dir eq 'h') # horizontal flip
  {
    ($rotate{'u'}, $rotate{'l'}, $rotate{'d'}, $rotate{'r'}) = ($rotate{'u'}, $rotate{'r'}, $rotate{'d'}, $rotate{'l'});
    next;
  };
  die ("I didn't recognize $dir. RLHVUD (case insensitive) are the only directions I do.");
}

for $dir (sort keys %rotate)
{
  #print "$dir becomes $rotate{$dir}\n";
  $anyDif += ($dir ne $rotate{$dir});
}

#print "Rotation: URDL => $rotate{'u'}$rotate{'r'}$rotate{'d'}$rotate{'l'}\n";

if (!$anyDif) { die("The rotations you requested don't change the puzzle's orientation."); }

open(A, "sok.txt") || die ("No sok.txt.");

while (my $line = <A>)
{
  print "Orig: $line";
  print "NEW:: ";
  $line =~ s/\b([0-9\*]*)([URDL])\b/$1 . newdir($2)/gei; # I included * with numbers because it means "push to the end"
  chomp($line);
  print "$line\n"; #this is to avoid orig/new appearing on the same line at the end of a file.
}
close(A);

##################################subroutine

sub newdir
{
  if ($_[0] eq lc($_[0]))
  {
    return $rotate{$_[0]};
  }
  return uc($rotate{lc($_[0])});
}

Please check for additional comments in the code,

use strict;
use warnings;

# no need to die() when open()
use autodie;

# this hash transforms a move to a different direction. By default each move points to itself.
my %rotate = (
  'u' => 'u',
  'r' => 'r',
  'd' => 'd',
  'l' => 'l'
);

if (!defined($ARGV[0]))
{
  die ("You need a command line argument (no spaces) to use rotations: R, L, H and V/U/D rotate right, left, horizontally and vertically.");
}

my @adjust = split(//, lc($ARGV[0]));
#/(fix stackexchange code coloring)

for my $dir (@adjust) {
  my @keys;

  # rotate right
  if ($dir eq 'r') { @keys = qw(r d l u); }
  # rotate left
  elsif ($dir eq 'l') { @keys = qw(l u r d); }
  # vertical flip
  elsif ($dir eq 'v' or $dir eq 'u' or $dir eq 'd') { @keys = qw(d r u l); }
  # horizontal flip
  elsif ($dir eq 'h')  { @keys = qw(u l d r); }
  else {
    die ("I didn't recognize $dir. RLHVUD (case insensitive) are the only directions I do.");
  }

  # check "hash slice"
  @rotate{@keys} = @rotate{qw(u r d l)};
}

my $anyDif; # checks for differences once we've parsed rotations
for my $dir (keys %rotate) {
  #print "$dir becomes $rotate{$dir}\n";
  $anyDif += ($dir ne $rotate{$dir});
}

#print "Rotation: URDL => $rotate{'u'}$rotate{'r'}$rotate{'d'}$rotate{'l'}\n";

if (!$anyDif) { die("The rotations you requested don't change the puzzle's orientation."); }

# check "three argument open"
open(my $A, "<", "sok.txt");

while (my $line = <$A>) {
  print "Orig: $line";
  print "NEW:: ";
  $line =~ s/\b([0-9\*]*)([URDL])\b/$1 . newdir($2)/gei; # I included * with numbers because it means "push to the end"
  chomp($line);
  print "$line\n"; #this is to avoid orig/new appearing on the same line at the end of a file.
}
close($A);

##################################subroutine

sub newdir {
  my ($v) = @_;
  my $lc = lc($v);

  # check "ternary operator"
  return ($v eq $lc) ? $rotate{$v} : uc($rotate{$lc});
}


Project Euler #2 in Haskell Problem

By considering the terms in the Fibonacci sequence whose values do not
exceed four million, find the sum of the even-valued terms.

My solution

-- Formula for finding n-th Fibonacci number
fibonacci 1 = 1
fibonacci 2 = 2
fibonacci n
    | n >= 3    = fibonacci (n - 1) + fibonacci (n - 2)
    | otherwise = fibonacci 2 + fibonacci 1

-- Intermediate code: list of first 32 Fibonacci numbers
fibonacciList = [fibonacci i | i <- [1..32]]

-- list of even Fibonacci numbers from fibonacciList
evenFibonacci = [eF | eF <- fibonacciList, eF `mod` 2 == 0]

-- list of evenFibonacci from fibonacciList not exceeding 4 million
evenFibonacci' = [eF' | eF' <- evenFibonacci, eF' <= 4000000]

-- sum of the even fibonacci numbers not exceeding 4 million
sumEvenFibonacci' = sum evenFibonacci'

-- display the result
main = print sumEvenFibonacci

Your fibonacci function is veryyyyyy slow.
Why? Well

fibonacci 5 = fibonacci 3 + fibonacci 4
            = fibonacci 1 + fibonacci 2 + fibonacci 2 + fibonacci 3
            = 1 + 2 + 2 + fibonacci 1 + fibonacci 2
            = 8

Do you see what's wrong? That's right, you computed fibonacci 3 two times.
So, for high values of n, you are going to compute it a lot!
The naive linear fibonacci is usually written as fibonacci = 0 : 1 : (zipWith (+) fibonacci (tail fibonacci)) and is explained here: https://stackoverflow.com/questions/43486880/take-n-with-zipwith-and-tail

Another tip: Do not overuse list comprehension, it tends to get very unreadable fast. For instance your:

evenFibonacci' = [eF' | eF' <- evenFibonacci, eF' <= 4000000]

Is misleading because it wouldn't work if evenFibonacci is infinite (but it really should). I believe that is why you declared fibonacciList as finite, while there wasn't any reason to.
So how could we make it so it does? Well we know evenFibonacci is strictly increasing, so we could take advantage of it:

evenFibonacci' = takeWhile (<= 40000) evenFibonacci

Similarilly fibonacci <$> [1 .. 32] is even shorter than your version.
(I'll let you rewrite evenFibonacci on your own ;) )

Finally I advise you not to do so many variable declarations, declarations are fine when those variables can be standalones, but it isn't really the case of your evenFibonacci'. You really should either compose variables or use some where

Have fun!


tablename alias in attribute name I have dataframe like this:

date        tableName                attributeName
2019-03-29  [order as o, costumer]   [costumer.customerID, o.orderID]
2019-03-30  [customer c, payment]    [payment.paymentID, c.firstName]
...

I have a function that map the tableName to attribute, like this:

import pandas as pd
import re
def countTableAttribute(dataFrame, tableName, attributeName):
    a       = (dataFrame[tableName].values.tolist())
    r       = re.compile(r'\b(as|\s)\b',re.IGNORECASE)
    alias   = list((x,n) for x in range(len(a)) for n in a[x] if bool(r.search(n)))
    df3     = (pd.DataFrame(dataFrame[tableName].values.tolist(), index=dataFrame.index).stack().str.split(' as | ', expand=True)).dropna()
    if alias != []:
        d   = dict(zip(df3[1], df3[0]))
    else:
        d   = dict(zip(df3[0], df3[0]))
    dfs     = pd.DataFrame(columns=[-1,0,1])
    for i in range(len(dataFrame)):
        for x in dataFrame[attributeName][i]:
            if not '.' in x:
                ser = pd.Series([dataFrame['date'][i],dataFrame[tableName][i][0], x], index=dfs.columns)
                dfs = dfs.append(ser, ignore_index=True)
            else:
                ser = pd.Series([dataFrame['date'][i],x.split('.')[0], x.split('.')[1]], index=dfs.columns)
                dfs = dfs.append(ser, ignore_index=True)
    dfs.columns         = ['Date','tableName','attributeName']
    dfs['tableName']    = dfs['tableName'].replace(d)
    dfs                 = dfs.groupby(['Date','tableName','attributeName'], sort=False).size().reset_index(name='Count')
    return dfs

so the output is like this:

   date        tableName   attributeName  count
2019-03-29     order       orderID          1
2019-03-29     costumer    customerID       1
2019-03-30     customer    firstName        1
2019-03-30     payment     paymentID        1

But as this is my first try, I need an opinion about what I've tried, because my code runs slow.

Thank you

Some suggestions:

black can format your code to be more idiomatic.
flake8 can check your code for remaining non-idiomatic code.
mypy can enforce typing to make it more obvious what the code is meant to do.
Names like a, r, d and dfs are unhelpful. Naming is a hard skill to learn, but is enormously helpful in making code more readable and therefore maintainable.
if not '.' in x: etc looks like it might be a good candidate for a separate function - it doesnt't take too many inputs, returns only one thing (dfs), and is short.
It looks like you are doing a lot of conversions: dataFrame[tableName].values.tolist(), list(…), dict(). I'm not familiar with Pandas, but I expect you can speed things up significantly by not converting values.


Algorithm with byte shifts and BCD calculation I'm trying to develop an algorithm that generates a 16-digit validation number to interact with a given system.

This is how the algorithm should be implemented:

I have implemented it, and have it working with 2 of 3 test cases that I have found (I will post them below).

The implementations goes as:

public static String Generate(byte[] _MachineId, byte[] _SN)
{
        try
        {
            byte[] step1 = new byte[6]
            {
                _SN[2],
                _SN[1],
                _SN[0],
                _MachineId[2],
                _MachineId[1],
                _MachineId[0]
            };

            byte[] step2 = new byte[6]
            {
                step1[0],
                step1[1],
                (byte)(step1[2] ^ step1[0]),
                (byte)(step1[3] ^ step1[1]),
                (byte)(step1[4] ^ step1[0]),
                (byte)(step1[5] ^ step1[1])
            };

            var crc45 = new byte[2] { step2[4], step2[5] }.GetCRCAsByteArray(0);
            var crc23 = new byte[2] { step2[2], step2[3] }.GetCRCAsByteArray(0);
            var crc01 = new byte[2] { step2[0], step2[1] }.GetCRCAsByteArray(0);

            byte[] step3 = new byte[6]
            {
                crc01[0],
                crc01[1],
                crc23[0],
                crc23[1],
                crc45[0],
                crc45[1]
            };

            string bin_to_BCD_543 = ((uint)((((0x00 << 4) | step3[5]) << 16) | ((step3[4] << 8) | step3[3]))).ToString().PadLeft(8, '0');
            string bin_to_BCD_210 = ((uint)((((0x00 << 4) | step3[2]) << 16) | ((step3[1] << 8) | step3[0]))).ToString().PadLeft(8, '0');

            String step4pre = bin_to_BCD_543 + bin_to_BCD_210;
            Func<String, int[]> convertToIntArray =
                    new Func<string, int[]>(
                    (str) =>
                    {
                        List<int> result = new List<int>();
                        foreach (var c in str.ToCharArray()) result.Add(int.Parse(c.ToString()));
                        return result.ToArray();
                    }
                );
            int[] step4 = convertToIntArray(step4pre);

            int[] step5 = step4.ToList().ToArray();

            step5[8] = step4[8] | ((step4.ToList().GetRange(8, 8).Sum() % 5) << 1);
            step5[0] = step4[0] | ((step4.ToList().GetRange(0, 8).Sum() % 5) << 1);

            String validation = String.Join("", step5);
            return validation;
        }
        catch (Exception exc)
        {
            System.Diagnostics.Trace.TraceError("Error generating Validation number. Error => " + exc.ToString());
        }
        return "";
    }

This, are the 2 test cases that I have working:

ID = {0x21, 0x43, 0x65}, SN = {0x01, 0x00, 0x00} -> VN = "6429188185446104"
ID = {0x21, 0x43, 0x65}, SN = {0x02, 0x00, 0x00} -> VN = "2701773123879856"

I have found a 3rd case in another document, that should satisfy this:

ID = {0x01, 0x00, 0x10}, SN = {0x99, 0x00, 0x00} -> VN = "0230633823748784"

But this one does not works for me, so my questions are:

Is the algorithm implementation OK in terms of satisfying the requirements?
How can this implementation be improved?

The algorithm seems to be fine, and if it does not fail the tests it should be OK (try to get more test cases though).

Regarding the implementation:

Indentation - not sure if it just the site or you deliberately placed your curly braces the way they are placed, but it they certainly look weird, especially with the array initialization. You should indent when there is a new block, no matter what.
Use var in place of explicit types - makes reading and refactoring easier.
Argument names - In the C# world, the _variable naming conventionally means a private member variable. Even if you choose not to honor that convention, your argument names are very confusing. Instead of 1-2 letter abbreviations you should use descriptive names.
Too long - split into multiple methods. It doesn't matter that they are only called once: descriptive function names will give you a "bird's view" of the algorithm. Also, you should not use Func just because you can; it's cool, but code is better when not cool but clear, so turn that into a normal method.
Explain the padding part in a comment - it is actually a nice solution, but a bit counter-intuitive (my first thought was: why would you convert to string to just convert back?, but it's actually cleaner than filling the padding zeros by hand).
step4.ToList().ToArray() - is a confusing (and half as fast) way of saying step4.Clone(). BTW it wouldn't hurt to just use Lists and avoid the other ToList calls as well.
I would argue that you don't need the try-catch block around the whole method, since the only thing that looks like it could throw is your CRC calls, so you should only handle that part. If anything else throws, it's better if it's ugly and noisy (a.k.a. don't catch it; clients who call your method will get the empty string back and no other indication of failure, which is not really a good practie.
Use XML documentation comments for the method to document purpose, return value, arguments.
Make sure that a public static method is appropriate for your needs. Instead of public, you might want to use internal. And remember, you can't override static methods in derived classes, so you can't take advantage of polymorphism later.


CLS_Compound_Predicate I have a CLS_Comparison_Predicate which will take an input value, perform a logical comparison and return a Boolean.

CLS_Compound_Predicate instead takes lists of predicates and an inputValue, and performs MatchAny() or MatchAll() operations on the predicate list.

E.G.

  predicate_1 has >= 3
  predicate_2 has = 5

  MatchAny(6) Returns True
  MatchAll(6) Returns False

Thoughts?

Option Explicit

Private predicateList As Variant

Private Const NULL_ERROR_TEXT As String = "Invalid Compare input. Cannot compare against Null"
Private Const OBJECT_ERROR_TEXT As String = "Invalid Compare input. Input must be a value, not an object"
Private Const NOTHING_ERROR_TEXT As String = "Invalid Compare Input. Predicate cannot be Nothing"
Private Const EMPTY_ERROR_TEXT As String = "Invalid Compare Input. Input cannot be empty"
Private Const ZLS_ERROR_TEXT As String = "Invalid Compare Input. Input cannot be a Zero-Length-String"

Public Sub AddPredicate(ByRef inputPredicate As CLS_Comparison_Predicate)

    If inputPredicate Is Nothing Then
        PrintErrorMessage NOTHING_ERROR_TEXT
        Stop
    End If

    If IsEmpty(predicateList) Then
        ReDim predicateList(1 To 1)
        Set predicateList(1) = inputPredicate
    Else
        Dim LB1 As Long, UB1 As Long
        GetBounds LB1, UB1

        ReDim Preserve predicateList(LB1 To UB1 + 1)
        Set predicateList(UB1 + 1) = inputPredicate
    End If

End Sub

Public Sub ClearPredicates()

    If Not predicateList Is Nothing Then Erase predicateList

End Sub

Public Function MatchAny(ByVal leftValue As Variant) As Boolean
    '/ compares leftValue against predicateList. If any return true, then return true, else return false

    CheckInputValue leftValue

    Dim LB1 As Long, UB1 As Long
    GetBounds LB1, UB1

    Dim matchesAny As Boolean
    matchesAny = False

    Dim predicate As CLS_Comparison_Predicate
    Dim ix As Long
    For ix = LB1 To UB1
        Set predicate = predicateList(ix)
        If predicate.Compare(leftValue) Then
            matchesAny = True
            Exit For
        End If
    Next ix

    MatchAny = matchesAny

End Function

Public Function MatchAll(ByVal leftValue As Variant) As Boolean
    '/ compares leftValue against predicateList. If any return false, then return false, else return true

    CheckInputValue leftValue

    Dim LB1 As Long, UB1 As Long
    GetBounds LB1, UB1

    Dim matchesAll As Boolean
    matchesAll = True

    Dim predicate As CLS_Comparison_Predicate
    Dim ix As Long
    For ix = LB1 To UB1
        Set predicate = predicateList(ix)
        If Not predicate.Compare(leftValue) Then
            matchesAll = False
            Exit For
        End If
    Next ix

    MatchAll = matchesAll

End Function

Private Sub CheckInputValue(ByVal inputValue As Variant)
    '/ Check for NULL, Objects, Empty and ZLS

    If IsNull(inputValue) Then
        PrintErrorMessage NULL_ERROR_TEXT
        Stop
    End If

    If IsObject(inputValue) Then
        PrintErrorMessage OBJECT_ERROR_TEXT
        Stop
    End If

    If IsEmpty(inputValue) Then
        PrintErrorMessage EMPTY_ERROR_TEXT
        Stop
    End If

    On Error Resume Next
        If Len(inputValue) = 0 Then
            PrintErrorMessage ZLS_ERROR_TEXT
            Stop
        End If
    On Error GoTo 0

End Sub

Private Sub GetBounds(Optional ByRef LB1 As Long, Optional ByVal UB1 As Long)

    LB1 = LBound(predicateList)
    UB1 = UBound(predicateList)

End Sub

I learned something from this

On Error Resume Next
    If Len(inputValue) = 0 Then
        PrintErrorMessage ZLS_ERROR_TEXT
        Stop
    End If
On Error GoTo 0

I learned that On Error GoTo 0 disables error handling in the current procedure. The KB also says:

  Without an On Error GoTo 0 statement, an error handler is
  automatically disabled when a procedure is exited.

So you really don't need it. I also can't figure out under what conditions the len would error. The only thing I could think of is if it's an object, but you've exited by that point. It can't be a negative length can it?


User-defined color implementation in windows console I have implemented an user defined colors enumerator, and if is possible I appreciate if I will get a code review.
I want to know if my implementation is correctly or must improve it.
If there exist a better alternative (color enumerator) to my code which is?

Usage example:

 HANDLE hConsole;
 setTextColor(hConsole,YELLOW_TEXT|PINK_BACKGROUND);
 std::cout << "YELLOW_TEXT|PINK_BACKGROUND";
 resetTextColor(hConsole);

Colors enumerator:

 enum COLOR
 {
     // Text foreground colors
     // Standard text colors
     GRAY_TEXT=8,      BLUE_TEXT,       GREEN_TEXT,
     TEAL_TEXT,        RED_TEXT,        PINK_TEXT,
     YELLOW_TEXT,      WHITE_TEXT,
     // Faded text colors
     BLACK_TEXT=0,     BLUE_FADE_TEXT,  GREEN_FADE_TEXT,
     TEAL_FADE_TEXT,   RED_FADE_TEXT,   PINK_FADE_TEXT,
     YELLOW_FADE_TEXT, WHITE_FADE_TEXT,
     // Standard text background color
     GRAY_BACKGROUND=GRAY_TEXT<<4,     BLUE_BACKGROUND=BLUE_TEXT<<4,
     GREEN_BACKGROUND=GREEN_TEXT<<4,   TEAL_BACKGROUND=TEAL_TEXT<<4,
     RED_BACKGROUND=RED_TEXT<<4,       PINK_BACKGROUND=PINK_TEXT<<4,
     YELLOW_BACKGROUND=YELLOW_TEXT<<4, WHITE_BACKGROUND=WHITE_TEXT<<4,
     // Faded text background color
     BLACK_BACKGROUND=BLACK_TEXT<<4,           BLUE_FADE_BACKGROUND=BLUE_FADE_TEXT<<4,
     GREEN_FADE_BACKGROUND=GREEN_FADE_TEXT<<4, TEAL_FADE_BACKGROUND=TEAL_FADE_TEXT<<4,
     RED_FADE_BACKGROUND=RED_FADE_TEXT<<4,       PINK_FADE_BACKGROUND=PINK_FADE_TEXT<<4,
     YELLOW_FADE_BACKGROUND=YELLOW_FADE_TEXT<<4, WHITE_FADE_BACKGROUND=WHITE_FADE_TEXT<<4
 };

Set and reset functions:

  BOOL resetTextColor(HANDLE h)
  {
      return SetConsoleTextAttribute(h,WHITE_FADE_TEXT);
  }

  BOOL setTextColor(HANDLE h, WORD c)
  {
      return SetConsoleTextAttribute(h,c);
  }

Capture:

If there exist a better alternative (color enumerator) to my code which is?

You should use the constants defined in a Windows header file named Wincon.h, which are as follows:

#define FOREGROUND_BLUE      0x0001 // text color contains blue.
#define FOREGROUND_GREEN     0x0002 // text color contains green.
#define FOREGROUND_RED       0x0004 // text color contains red.
#define FOREGROUND_INTENSITY 0x0008 // text color is intensified.
#define BACKGROUND_BLUE      0x0010 // background color contains blue.
#define BACKGROUND_GREEN     0x0020 // background color contains green.
#define BACKGROUND_RED       0x0040 // background color contains red.
#define BACKGROUND_INTENSITY 0x0080 // background color is intensified.

For example, you could write:

enum COLOR
{
    // Text foreground colors
    // Standard text colors
    GRAY_TEXT=FOREGROUND_INTENSITY,
    BLUE_TEXT=FOREGROUND_BLUE,

The answers to C++ Win32 Console Color should be useful to you.

Alternatively here's a copy-and-paste of the equivalent .NET enum for C#, which I believe uses the same bit-values and assigns conventional color-names:

#region Assembly mscorlib.dll, v4.0.30319
// C:\Program Files\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\Profile\Client\mscorlib.dll
#endregion

namespace System
{
    // Summary:
    //     Specifies constants that define foreground and background colors for the
    //     console.
    [Serializable]
    public enum ConsoleColor
    {
        // Summary:
        //     The color black.
        Black = 0,
        //
        // Summary:
        //     The color dark blue.
        DarkBlue = 1,
        //
        // Summary:
        //     The color dark green.
        DarkGreen = 2,
        //
        // Summary:
        //     The color dark cyan (dark blue-green).
        DarkCyan = 3,
        //
        // Summary:
        //     The color dark red.
        DarkRed = 4,
        //
        // Summary:
        //     The color dark magenta (dark purplish-red).
        DarkMagenta = 5,
        //
        // Summary:
        //     The color dark yellow (ochre).
        DarkYellow = 6,
        //
        // Summary:
        //     The color gray.
        Gray = 7,
        //
        // Summary:
        //     The color dark gray.
        DarkGray = 8,
        //
        // Summary:
        //     The color blue.
        Blue = 9,
        //
        // Summary:
        //     The color green.
        Green = 10,
        //
        // Summary:
        //     The color cyan (blue-green).
        Cyan = 11,
        //
        // Summary:
        //     The color red.
        Red = 12,
        //
        // Summary:
        //     The color magenta (purplish-red).
        Magenta = 13,
        //
        // Summary:
        //     The color yellow.
        Yellow = 14,
        //
        // Summary:
        //     The color white.
        White = 15,
    }
}

If you're going to define your own enum then use it in your function declaration, i.e.:

BOOL setTextColor(HANDLE h, COLOR c)

... not ...

BOOL setTextColor(HANDLE h, WORD c)

I'd usually use mixed-case e.g. Color instead of COLOR for an enum; and I'd worry that it's already been defined in a header file.

Also I'd split it into two parameters instead of doubling the number of enum values, for example:

BOOL setTextColor(HANDLE h, ConsoleColor foregound, ConsoleColor background)
{
    WORD color = foregound | (background << 4);
    return SetConsoleTextAttribute(h, color);
}


Correct way of using HBRUSHes..? If I create a brush like so

HBRUSH hBrush = CreateSolidBrush(RGB(33, 33, 33));

and then later on, I want to change the color of the brush, how would I do this?
I googled a bit and found out that creating a bunch of different brushes is not the way to go.

I was thinking that one possibility would be to just set hBrush to a new brush like so

hBrush = CreateSolidBrush(RGB(0, 0, 0));

everytime I wanted to use a different color, but this seems suspiciously easy, and I'm pretty sure that this would hog resources or something.

Another - presumably bad - idea of mine, was to use the above method, but always use

DeleteObject(hBrush);

immediately after drawing. I have no idea if this is the right way to do things or not. Any help would be greatly appreciated.

Assume any Win32 call that returns a handle allocates a struct with malloc. And assume DeleteObject (DestroyWindow etc) frees allocated memory. you should call DeleteObject every time you creare a GDI object.

Calling CreateSolidBrush twice, and losing the first handle without calling Deleteobject on it causes a resource leak.

Since you are using C++ the correct way to do it is RAII. see http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

This site has a good tutorial with clear windows programming examples on the subject.
http://www.relisoft.com/book/win/index.htm

This is a good open source library (MIT licence), that you can use in your project directly. Or you can look at gdi.h in the source.
http://win32-framework.sourceforge.net/index.htm


Sorting by patients' name or birth Is there a way to write this simpler?

if(criteria.SortBy =="name")
{
    if (criteria.SortDirection == "ascending")
    {
        physicians.Sort((x, y) => string.Compare(x.Name, y.Name));
    }
    else
    {
        physicians.Sort((x, y) => string.Compare(y.Name,x.Name ));

    }
}

if (criteria.SortBy == "birth")
{
    if (criteria.SortDirection == "ascending")
    {
        physicians.Sort(new Comparison<Physician>((x, y) => DateTime.Compare(x.DateOfBirth ?? DateTime.MinValue,y.DateOfBirth ?? DateTime.MinValue)));
    }
    else
    {
        physicians.Sort(new Comparison<Physician>((x, y) => DateTime.Compare(y.DateOfBirth ?? DateTime.MinValue, x.DateOfBirth ?? DateTime.MinValue)));

    }
}

You can use the ComparerExtensions library to help you create comparers more easily.

With it, your code could look something like:

IComparer<Physician> comparer = null;

if (criteria.SortBy == "name")
{
    comparer = KeyComparer<Physician>.OrderBy(x => x.Name);
}
else if (criteria.SortBy == "birth")
{
    comparer = KeyComparer<Physician>.OrderBy(x => x.DateOfBirth);
}

if (comparer != null)
{
    if (criteria.SortDirection != "ascending")
    {
        comparer = comparer.Reversed();
    }

    physicians.Sort(comparer);
}


Perl script to check disk usage I'm looking for input on the code itself and formatting/style.  This script parses the output of df -h on a Linux server and emails the user if the usage percentage is above the threshold (90% in this case).

#!/usr/bin/perl

use warnings;
use strict;
use Mail::Sendmail;

my $svr = `hostname`;
my @output = `df -h`;
my $msg;
my $mailto = 'someone@foo.com';
my $threshold = 90;

sub main
{
  foreach (@output) {
    if (($_ =~ m/(\d+)% (.*)/) && ( $1 > $threshold )) {
      chomp($svr); $msg = "$svr: $2 volume  is $1 percent full\n" }
    }

    my %mail = ( To      => $mailto,
                 From    => 'Server@foo.com',
                 Subject => "$svr has a full file system",
                 Message => $msg
               );

  if ( defined $msg ) {
    sendmail(%mail) or die $Mail::Sendmail::error;
  }

}

main

There is a bug here:

foreach (@output) {
    if (($_ =~ m/(\d+)% (.*)/) && ( $1 > $threshold )) {
      chomp($svr); $msg = "$svr: $2 volume  is $1 percent full\n" }
    }

If there are more than one partitions full, $msg will contain only the last one. I think you want to append to $msg instead of overwriting. I suggest to rewrite like this:

chomp(my $svr = `hostname`);

foreach (@output) {
    if (m/(\d+)% (.*)/ && $1 > $threshold) {
        $msg .= "$svr: $2 volume is $1 percent full\n";
    }
}

Explanation of other improvements:

Removed unnecessary brackets from the if condition
Simplified $_ =~ m// to just m// which does the same
Instead of chomp($svr) for each line, do it only once, right after initialization
Reindented, so it's easier to see the block of code that belongs to the loop

Lastly, you can drop "defined" in if ( defined $msg ) {


Find factors of a number I am a beginner in C. I have just written this program to find the factors of a provided number \$n\$ where \$1\leq n \leq 10^9\$. However, when I input large numbers (e.g. the maximum, \$10^9\$), the program takes a long time to finish finding the larger factors. How do I reduce its time taken? Also, are there any possible improvements for this code?

#include<stdio.h>
int main(){
    int a,i;
    scanf("%d",&a);
    for(i=1;i<(a/2+1);i++){
        if(a%i==0){
            printf("%d\n",i);
        }
    }
    printf("%d\n",a);
    return 0;
}

You should realize that if \$i\$ is a divisor of \$a\$, then so is \$a / i\$. This way you get two divisors per found divisor.

In the same vein, you only need to search up to \$\sqrt{n}\$, because all factors above that have already been found as the second divisor.

So your code would become:

#include <stdio.h>
#include <math.h>

int main(){
        int a,i;
        scanf("%d",&a);
        int bound = ceil(sqrt(a));
        for(i=1; i <= bound; i++) {
                if(a%i==0) {
                        printf("%d\n",i);
                        if(a/i != i) printf("%d\n", a/i);
                }
        }
        printf("%d\n",a);
        return 0;
}


Find index of double number delimiter I have a String with a double number. Unfortunately, the number is created on backends with different locals, so it could be both 101.02 and 101,02 (different delimiters). I need to get the position of this delimiter if it exists and get 0, if it is not.

I've come to two options:

int pos = amount.indexOf(',') == -1 ?
                (amount.indexOf('.') == -1 ? 0 : amount.indexOf('.'))
                : amount.indexOf(',');

Second option with the same logic but different style:

int pos = amount.indexOf(',');
if (pos == -1) pos = amount.indexOf('.');
if (pos == -1) pos = 0;

I do not need to have a double number from String, I just need the position of the delimiter to color the String (using the Android class Spannable).

Is there a cleaner way to achieve this goal? And which of these styles are better, in your opinion? Is there some way to use the DecimalFormat class to achieve the goal?

The problems with the first approach

int pos = amount.indexOf(',') == -1 ?
                (amount.indexOf('.') == -1 ? 0 : amount.indexOf('.'))
                : amount.indexOf(',');

are that:

It's not that easily readable; the ternary operator has value when what is tested is simple enough. But when you start nesting them, it often degrades clarity.
The index is calculated two times, one to test whether it is -1 or not, and the second time to return the value.

As such, the second approach

int pos = amount.indexOf(',');
if (pos == -1) pos = amount.indexOf('.');
if (pos == -1) pos = 0;

is the most preferable between the two, mainly for clarity. You should consider putting that into a utility method.

There would be other approaches like using a regular expression, but another good one would to not traverse the string potentially 2 times, and instead of looking whether the string has a certain character, loop through each character and see if it is one of the potential delimiters. With Java 8, you could have

int pos = amount.chars().filter(c -> c == '.' || c == ',').findFirst().orElse(0);

And you could write explicitly the for loop for Java ≤ 7.

Final point: having a position of 0 when neither , nor . are present in the String can be confusing; 0 is a valid index value for a string, and it can imply that the delimiter was the first character of the string. If you consider ".25" (that could be a valid representation of a double number, the 0 before being implied), the code would consider this as if having no delimiter.


ASP.net custom user control design I've built a custom control that generates a tab style navigation.  Each page might have different tabs to display, the aim was to modularise it so I can make global changes to all the tabs.

TabMenu.ascx

<%@ Control Language="C#" AutoEventWireup="true" CodeFile="TabMenu.ascx.cs" Inherits="TabMenu" %>

<asp:Panel runat="server" ID="TabMenuWrapper" CssClass="tabWrapper">

</asp:Panel>

TabMenu.ascx.cs

using System;
using System.Collections.Generic;
using System.Web;
using System.Web.UI;
using System.Web.UI.WebControls;
using System.Collections;

public partial class TabMenu : System.Web.UI.UserControl
{
    public string TabGroup { get; set; }    // The tab group this control belongs to
    public int SelectedTab { get; set; }    // Index of selected tab

    protected void Page_Load(object sender, EventArgs e)
    {
        ArrayList tabCollection = new ArrayList();
        MenuTab myTab;

        // Artorking tab group
        if (this.TabGroup.ToLower() == "artwork")
        {
            myTab = new MenuTab() { linkURL = "artworkHome.aspx", linkText = "First!" };
            tabCollection.Add(myTab);

            myTab = new MenuTab() { linkURL = "artworkHome.aspx", linkText = "Second!" };
            tabCollection.Add(myTab);

            myTab = new MenuTab() { linkURL = "artworkHome.aspx", linkText = "3rd!" };
            tabCollection.Add(myTab);

            myTab = new MenuTab() { linkURL = "artworkHome.aspx", linkText = "Fourth!" };
            tabCollection.Add(myTab);
        }

        // Add tabs to the page
        for (int i = 0; i < tabCollection.Count; i++)
        {
            MenuTab thisTab = ((MenuTab)(tabCollection[i]));
            thisTab.CreateTab();

            if (i == this.SelectedTab)
            {
                thisTab.tabPanel.CssClass = "tab tabSelected";
            }

            TabMenuWrapper.Controls.Add(thisTab.tabPanel);
        }
        TabMenuWrapper.Controls.Add(new Panel(){CssClass = "clear"});

    }
}

// A menu tab
public class MenuTab
{
    public string linkURL;
    public string linkText;
    public HyperLink tabLink;
    public Panel tabPanel;

    // Create internal controls
    public void CreateTab()
    {
        this.tabLink = new HyperLink() { NavigateUrl = this.linkURL, Text = this.linkText };
        this.tabPanel = new Panel() { CssClass = "tab" };
        this.tabPanel.Controls.Add(this.tabLink);
    }
}

Used as follows:

<CrystalControls:TabMenu runat="server" ID="TopMenu" TabGroup="Artwork" SelectedTab="1" />

And renders like:

  First!

  Second!

  3rd!

  Fourth!

Is this a good design?

It might be better to render the control as an unordered list.  This is more semantically correct and normal practice for menu-type controls (which tabs are, if you think about it).


VBA macro to move data across workbooks I'm new to VBA and am trying to write a macro to move data from an exported file to an excel template. The code below works, however I have to add many more ranges of data and am worried it will be hefty to run.

Sub MoveData()

    Dim fileName As String
        Sheets("Cover").Select                                  '
        range("B5").Select
        fileName = Selection.Value
    Dim path As String
        path = "C:\Users\(name)\Documents\(folder)\" & fileName & ".csv"

    Dim currentWB As Workbook
    Set currentWB = ThisWorkbook

    Dim openWB As Workbook
    Set openWB = Workbooks.Open(path)

    Dim openWs As Worksheet
    Set openWs = openWB.Sheets(fileName)

    openWs.range("C2:C51").Copy
    openWB.Close (False)
    currentWB.Worksheets("Bms").Activate
    range("C7:C56").Select
    ActiveSheet.Paste

End Sub

I took the base code from here(the closed section) and modified it to fit my needs. The file I am extracting data from will change, but will always be in the same folder, which is why I set up the path as such. I am moving the data from C2 and downwards to C7-down on the template. C56 is currently the last used cell on the template, however that is subject to change so I would prefer to get rid of the lower bound.

If there is a better way to write this, I would love some input. Since I'm still new at this, if you could explain your code as well, that would be very beneficial.

It's a nitpicky thing, but I prefer explicit scoping.

Sub MoveData()

Should be

Public Sub MoveData()

There are lots of reasons to avoid Activating and Selecting objects. Performance is just one of them.

Dim fileName As String
    Sheets("Cover").Select                                  '
    range("B5").Select
    fileName = Selection.Value

Would be better written as

Dim fileName As String
fileName = Worksheets("Cover").Range("B5").Value

Note that I used the Worksheets collection instead of Sheets. This is because Sheets can have charts, Worksheets can't.

Dim currentWB As Workbook
Set currentWB = ThisWorkbook

Good call and good naming. Well done.

It's very slow to open a workbook. Luckily, you might not even have to open the book with the source data. I would look into querying the Excel data source with ADODB instead. Once you have a recordset, you can use the CopyFromRecordset method of Range to paste into the current workbook. It's much faster than loading the data source into Excel.

Here's some untested code to get you started.

Dim cn as ADODB.Connection
Set cn = New ADODB.Connection
With cn
    .Provider = "Microsoft.Jet.OLEDB.4.0"
    .ConnectionString = "Data Source=" & path & ";" & _
"Extended Properties=Excel 14.0;"
    .Open
End With

strQuery = "SELECT * FROM [Sheet1$C2:C51]"

Dim rst As New ADODB.Recordset
rst.Open sqlQuery, cn

currentWB.Worksheets("Bms").Range("C7").CopyFromRecordset rst

I don't think you understand what parenthesis do in procedure calls.

openWB.Close (False)

There's no harm in passing a boolean literal ByVal in this case, but make sure you save yourself the headache down the road and go read that StackOverflow Q&A I linked to. Parenthesis change how the argument is passed to a procedure.


Logic/Code duplication, improve readability? I have a function that takes a point, and given its velocity, and acceleration, calculates the time(s) for the point to collide with the line:

def ParabolaLineCollision(pos, vel, acc, line):
    """A pure function that returns all intersections."""
    pass

I have another function that wants to calculate the smallest intersection time between two entities, where an entity has a velocity/acceleration, and a list of vertices.

The algorithm of entity_intersection creates lines for each objects' list of vertices, and then calls ParabolaLineCollision on each point of each object, with each line of the other object.  My current algorithm runs slowly, and thus I would like to be able to parallelize the algorithm.

My problem is that entity_intersections is a bit hard to read, and I think that there may be some code duplication, which might possibly be eliminated.  Here's the algorithm, and

class Entity
    # ...

    @property
    def lines(self):
        """
        Get a list of lines that connect self.points.

        If self.enclosed, then the last point will connect the first point.
        """
        # type(self.enclosed) == bool

        # # I included this, as this may be where there is logic duplication with entity_intersections.
        # # It's also not very readable.

        return [(self.points[i-1], self.points[i]) for i in range((1-int(self.enclosed)), len(self.points))]

    # ...

def entity_intersections(entity, other):
    """
    Find all intersection times between two entities.

    type(entity) == type(other) == Entity
    """
    # Find relative velocity/acceleration
    a12 = entity.acceleration - other.acceleration
    a21 = other.acceleration - entity.acceleration
    v12 = entity.velocity - other.velocity
    v21 = other.velocity - entity.velocity

    entity_points, other_points = entity.points, other.points
    entity_lines, other_lines = entity.lines, other.lines

    results = []

    # Get all intersections between each object's point and the other object's line
    for point in entity_points:
        for line in other_lines:
            results.append((point, v12, a12, line))
    for point in other_points:
        for line in entity_lines:
            results.append((point, v21, a21, line))

    # Return results of ParabolaLineCollision with each list of arguments in results
    # Pseudo-code to achieve this (I haven't done the parallizing yet):
    # return min(map(ParabolaLineCollision, results))

Not a lot of changes, but maybe a bit more legible.

a12 = entity.acceleration - other.acceleration
a21 = -a12
v12 = entity.velocity - other.velocity
v21 = -v12

result  = [ (p, v12, a12, l) for l in other.lines for p in entity.points ] +
          [ (p, v21, a21, l) for l in entity.lines for p in other.points ]

Obviously you could inline the v21 and a21 variables, but it's actually somewhat descriptive of what they represent to use those names.


Cheese-Burger-Waffles (aka Rock-Paper-Scissors) FizzBuzz was fun, and I got great feedback and learned a few things (which I hope I put in practice here), but only scratched the surface. I wanted to explore the LOLCODE language a bit more, so I implemented a little rock-paper-scissors, to play with functions, parameters, return values.. and user input.
Unfortunately I haven't figured out how to get compileonline to take more than a single value through STDIN, so I dropped the idea of making a GAEMLOOPZ and only made it a single round.
Also AFAICT there's no way of generating random numbers in LOLCODE (specs), so the LOLCAT always wins - it even has a seekrit move for invalid inputs!
HAI 1.2

  I HAS A CHEEZ ITZ "CHEEZ"       BTW "ROCK"
  I HAS A BURGR ITZ "BURGR"       BTW "PAPER"
  I HAS A WAFLZ ITZ "WAFLZ"       BTW "SCISSORS"
  I HAS A SEEKRITMOOV ITZ "ZPOCK" BTW SHH, SEEKRIT

  HOW IZ I GETAMOOV

    VISIBLE "WHAT U PLAY?"
    VISIBLE "[C]HEEZ [B]URGR [W]AFLZ"

    I HAS A MOOV
    GIMMEH MOOV

    MOOV R I IZ VALID8MOOV YR MOOV MKAY
    FOUND YR MOOV

  IF U SAY SO

  HOW IZ I VALID8MOOV YR MOOV

    MOOV, WTF?
      OMG "C", OMG "c"
        MOOV R CHEEZ
        GTFO

      OMG "B", OMG "b"
        MOOV R BURGR
        GTFO

      OMG "W", OMG "w"
        MOOV R WAFLZ
        GTFO

      OMGWTF
        ANY OF BOTH SAEM MOOV AN "ZPOCK" AN BOTH SAEM MOOV AN "zpock" MKAY
        O RLY?
          YA RLY, MOOV R SEEKRITMOOV
          NO WAI
            MOOV R SMOOSH "... WUTS " AN MOOV AN " ANYWAI" MKAY
        OIC

    OIC

    FOUND YR MOOV

  IF U SAY SO

  HOW IZ I PIKAMOOV YR MOOV

    I HAS A ULTIMAETMOOV

    MOOV, WTF?
      OMG "CHEEZ" BTW must be a constant value :(
        ULTIMAETMOOV R BURGR
        GTFO

      OMG "BURGR" BTW must be a constant value :(
        ULTIMAETMOOV R WAFLZ
        GTFO

      OMG "WAFLZ" BTW must be a constant value :(
        ULTIMAETMOOV R CHEEZ
        GTFO

      OMG "ZPOCK" BTW must be a constant value :(
        ULTIMAETMOOV R "Y U CHEAT? I"
        GTFO

      OMGWTF
        ULTIMAETMOOV R SEEKRITMOOV

    OIC

    FOUND YR ULTIMAETMOOV

  IF U SAY SO

  HOW IZ I WINTEHGAEM YR MOOV AN YR ULTIMAETMOOV

    VISIBLE SMOOSH "Y U PLAY " AN MOOV MKAY
    VISIBLE SMOOSH "MAH " AN ULTIMAETMOOV AN " WINZ LOL" MKAY

  IF U SAY SO

  OBTW
    was going to make a GAEMLOOP,
    but stupidly I can't seem to figure out
    how to pass multiple values to STDIN with this "IDE"...
    so this program will only run a single round.
  TLDR

  HOW IZ I PLAYAMOOV

    I HAS A MOOV
    MOOV R I IZ GETAMOOV MKAY

    I HAS A ULTIMAETMOOV
    ULTIMAETMOOV R I IZ PIKAMOOV YR MOOV MKAY

    I IZ WINTEHGAEM YR MOOV AN YR ULTIMAETMOOV MKAY

  IF U SAY SO

  VISIBLE "CHEEZ-BURGR-WAFLZ:)"
  I IZ PLAYAMOOV MKAY

KTHXBYE

Outputs
Input: c or C

CHEEZ-BURGR-WAFLZ

WHAT U PLAY?
[C]HEEZ [B]URGR [W]AFLZ
Y U PLAY CHEEZ
MAH BURGR WINZ LOL

Input: b or B

CHEEZ-BURGR-WAFLZ

WHAT U PLAY?
[C]HEEZ [B]URGR [W]AFLZ
Y U PLAY BURGR
MAH WAFLZ WINZ LOL

Input: w or W

CHEEZ-BURGR-WAFLZ

WHAT U PLAY?
[C]HEEZ [B]URGR [W]AFLZ
Y U PLAY WAFLZ
MAH CHEEZ WINZ LOL

Input: zzz ..or pretty much anything else:

CHEEZ-BURGR-WAFLZ

WHAT U PLAY?
[C]HEEZ [B]URGR [W]AFLZ
Y U PLAY ... WUTS zzz ANYWAI
MAH ZPOCK WINZ LOL

Input: zpock or ZPOCK

CHEEZ-BURGR-WAFLZ

WHAT U PLAY?
[C]HEEZ [B]URGR [W]AFLZ
Y U PLAY ZPOCK
MAH Y U CHEAT? I WINZ LOL

I don't like that I'm hard-coding the strings in the WTF (switch) block in VALID8MOOV, but I don't think it's possible to extract just the first letter of a YARN (string)... or is it? Also there doesn't seem to be a way of defining compile-time constants, so I had to put literal strings in PIKAMOOV as well.

HAI 1.2

    I HAZ A REVIEW ITZ "Following" BTW Let's roll ;)

I got a few points I want to make here and the first is actually quite simple. Don't name your names in INTORN3TZP34KZ...

Yes I know it's an esoteric language, and I know that all the instructions are defined to be "Internet-Speaks". But that doesn't mean you absolutely have to make your code follow the spec definitions.
(Who makes their Stack a class anyway..)

CHEEZ, BURGR, WAFLZ, SEEKRITMOOV, ... these names are suboptimal. (Pardon my non-french).

You had it pointed out to you in your previous question, that the documentation uses lowercase for variables. (Seems the specs changed that...)
I think you should adopt either that or at least anything
but SHOUTCASE! There is no need to give a darn about convention when convention is dumb ;)

Especially with the missing syntax highlighting it is hard to read your code and differentiate between instructions and identifiers.

Additionally to that I want to suggest using non-speakzified variable names. Simply because it's easier on the brain to not have to unwrap the variable name before being allowed to think about what the variable name means in the first place.

Moving swiftly on from names to... names..

PIKAMOVE - Is it just me or does that call associations with Pokémon to your head??
VALID8MOOV - What's the 8 anyways???

The names you chose are confusing. Might (wink wink) be because you actually made them Internetspeakz again :(

Next!

Whitespace / Newlines

    OIC

    FOUND YR ULTIMAETMOOV

  IF U SAY SO

  HOW IZ I WINTEHGAEM YR MOOV AN YR ULTIMAETMOOV

let's see how you'd put braces in Java (or C# for that matter) for this code:

    }
    return ultimateMoov;
}

void winthegaem (Object moov, Object ultimatemoov) {

You might want to additionally differentiate the "difference" between IF YOU SAY SO and HOW IZ I. I'd either add an additional newline between these or remove them on "closing the braces".

Comments

  OBTW
    was going to make a GAEMLOOP,
    but stupidly I can't seem to figure out
    how to pass multiple values to STDIN with this "IDE"...
    so this program will only run a single round.
  TLDR

Hmm.. Interesting. But completely irrelevant for the code This comment is not helping. You did mention it in your explanation, so in the code it's meaningless.

Reading the code does not become easier with reading this comment. Remove it.

OMGWTF

Y U NO?

    OMG "ZPOCK", OMG "zpock"
        MOOV R SEEKRITMOOV
        GTFO

    OMGWTF
        MOOV R SMOOSH "... WUTS " AN MOOV AN " ANYWAI" MKAY
  OIC

KTHXBYE

BTW End of review ;)