Merging word streams from files A follow-on, rags-to-riches implementation of The most efficient way to merge two lists in Java

The original requirements are to:

  Identify the distinct values from two input files, and output the
  distinct values to an output file. There is no specification for the
  order of the output, only that each line should be unique in the
  results.

  Special consideration should be made for efficiency.

I have implemented a more general specification:

merge multiple input files (at least one) to an output file
each line is treated as a line, not necessarily a "word". If the input files have just one word per line, then the output would be the same as the original specification.
take the input files from the commandline (the first file is the output file).

In my answer to the linked post I suggested that a Java 8 Streams implementation would be "nice". I have implemented that solution here. I am looking for suggestions on how to better utilize the new Java functionality, and any other suggestions you may have.

import java.io.BufferedWriter;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Stream;

@SuppressWarnings("javadoc")
public class Linemerge {

    /* Wrap the IOException in order to make convenient Stream usage. */
    private static final void writeWord(BufferedWriter writer, String word) {
        try {
            writer.write(word);
            writer.newLine();
        } catch (IOException e) {
            throw new UncheckedIOException(e);
        }
    }

    private static void merge(Path source, Set<String> seen, BufferedWriter writer) throws IOException {
        try (Stream<String> words = Files.lines(source)) {
            words.filter(seen::add).forEach(word -> writeWord(writer, word));
        }
    }

    public static void main(String[] args) {
        if (args.length < 2) {
            throw new IllegalArgumentException("Need at least two file arguments: Destination Source {Source {Source {...}}}");
        }
        Path dest = Paths.get(args[0]);
        try (BufferedWriter writer  = Files.newBufferedWriter(dest)) {
            Set<String> seen = new HashSet<>();
            for (int i = 1; i < args.length; i++) {
               Path source = Paths.get(args[i]);
               if (Files.isRegularFile(source) && Files.isReadable(source)) {
                   System.out.println("Merging " + source);
                   merge(source, seen, writer);
               } else {
                   System.out.println("Unable to read (and ignoring) " + source);
               }
            }
        } catch (IOException e) {
            e.printStackTrace();
            System.exit(1);
        }
    }

}

"Need at least two file arguments: Destination Source {Source {Source {...}}}"

I think an easier way of documenting that, at least for *nix, is:

"Need at least two file arguments: DESTINATION [SOURCE]..."

You can also turn the Paths in your main() method into a Stream too:

public class Linemerge {
    // ...

    // suggestion note: had to wrap IOException -> UncheckedIOException too
    private static void merge(Path source, Set<String> seen, BufferedWriter writer) {
        try (Stream<String> words = Files.lines(source)) {
            words.filter(seen::add).forEach(word -> writeWord(writer, word));
        } catch (IOException e) {
            throw new UncheckedIOException(e);
        }
    }

    private static final Predicate<Path> FILTER =
            f -> Files.isRegularFile(f) && Files.isReadable(f);

    private static void checkPath(Path path) {
        System.out.println((FILTER.test(path) ? "Merging"
                : "Unable to read (and ignoring)") + " " + path);
    }

    public static void main(String[] args) {
        if (args.length < 2) {
            throw new IllegalArgumentException(
                    "Need at least two file arguments: DESTINATION [SOURCE]...");
        }
        try (BufferedWriter writer = Files.newBufferedWriter(Paths.get(args[0]))) {
            Set<String> seen = new HashSet<>();
            Stream.of(args).skip(1).map(Paths::get).peek(Linemerge::checkPath)
                    .filter(FILTER).forEach(f -> merge(f, seen, writer));
        } catch (IOException e) {
            e.printStackTrace();
            System.exit(1);
        }
    }
}


Properly managing a collection from deserialized XML file throughout app's lifetime I'm developing a WinForms app to manage some settings and profiles for multiple video games. The list of video games is specified by the user, and I'm trying to figure out how to properly maintain that list/collection throughout the lifetime of my application.

I decided on an XML file to manage the games list which looks like this:

<?xml version="1.0" encoding="utf-8" ?>
<Games>
  <Game>
    <Name>NBA 2K13</Name>
    <FolderPath>C:\Games\NBA2K13</FolderPath>
  </Game>
  <Game>
    <Name>First Person Shooter</Name>
    <FolderPath>C:\Games\FPS</FolderPath>
  </Game>
</Games>

It was suggested to me that I should not continually read/write to the XML file; instead, I should load the XML file into a collection (I'm using serialization for this), let the app work off of the collection, and then persist the collection back to the XML file when the app is closed. For the game collection class (Games), I'm also treating it as a hybrid "repository" (see the Add() method).

I came up with the following classes for that:

public class Game
    {
        [XmlElement("Name")]
        public string Name { get; set; }
        [XmlElement("FolderPath")]
        public string FolderPath { get; set; }

        public Game() {}

        public Game(string name, string folder)
        {
            Name = name;
            FolderPath = folder;
        }
    }

[XmlRootAttribute("Games")]
    public class Games
    {
        public Games()
        {
            Items = new List<Game>();
        }

        [XmlElement("Game")]
        public List<Game> Items { get; set; }

        public void Add(Game game)
        {
            if (!Items.Any(g => g.Name.EqualsIgnoreCase(game.Name)))
            {
                if (Directory.Exists(game.FolderPath))
                    Items.Add(game);
                else
                    MessageBoxUtil.ShowError("Invalid directory entered.");
            }
            else
                MessageBoxUtil.ShowError("Game names must be unique. " + game.Name + " already exists.");
        }
    }

This code seems to work fine, and loading/persisting the data is working as planned (I'd still appreciate suggestions on whether this is a good approach). What I'm struggling with is how to maintain that collection throughout the app's lifecycle. Right now, there are 2 forms that need access to this collection (Games class). Should the Games class be made static since the Items collection is derived from a single XML file and could possibly be considered global data? Would it be better to implement the singleton pattern and pass around a single instance of Games?

Both a singleton solution and a static instance are acceptable.  What I would recommend is that you add a static factory to Games, like:

public static Games LoadFromFile(String xmlFile);

You would call this near the top level of your application and then you would pass it into any of your other code that requires it.  It's essentially dependency injection, in that you design the code that's going to use the Games object to have it injected it.  It makes testing easier and it makes the design more flexible.


Ruby Koans' Greed Task Having coded in Java and C# for quite some years, I'm currently learning Ruby. I'm working my way through the Ruby Koans tutorial. At some point, you are to implement a method that calculates the game-score of a dice-game called Greed.

I came up with this recursive Java/C#-like method. It passes all the supplied unit tests, so technically it's correct.

Now I'm wondering: Is this good Ruby code? If not, how would a "Rubyist" write this method? And possibly: Why? I'm also not so happy about the amount of duplicate code but can't think
of a better Rubyish way.

def score(dice)   #dice is an array of numbers, i.e. [3,4,5,3,3]
  return 0 if(dice == [] || dice == nil)

  dice.sort!

  return 1000 + score(dice[3..-1]) if(dice[0..2] == [1,1,1])
  return 600 + score(dice[3..-1]) if(dice[0..2] == [6,6,6])
  return 500 + score(dice[3..-1]) if(dice[0..2] == [5,5,5])
  return 400 + score(dice[3..-1]) if(dice[0..2] == [4,4,4])
  return 300 + score(dice[3..-1]) if(dice[0..2] == [3,3,3])
  return 200 + score(dice[3..-1]) if(dice[0..2] == [2,2,2])
  return 100 + score(dice[1..-1]) if(dice[0] == 1)
  return 50 + score(dice[1..-1]) if(dice[0] == 5)
  return 0 + score(dice[1..-1]);
end

Some background (if needed)

# Greed is a dice game where you roll up to five dice to accumulate
# points. A greed roll is scored as follows:
#
# * A set of three ones is 1000 points
#
# * A set of three numbers (other than ones) is worth 100 times the
#   number. (e.g. three fours is 400 points).
#
# * A one (that is not part of a set of three) is worth 100 points.
#
# * A five (that is not part of a set of three) is worth 50 points.
#
# * Everything else is worth 0 points.
#
#
# Examples:
#
# score([1,1,1,5,1]) => 1150 points
# score([2,3,4,6,2]) => 0 points
# score([3,4,5,3,3]) => 350 points
# score([1,5,1,2,4]) => 250 points
#
# More scoring examples are given in the tests below:

class AboutScoringProject < EdgeCase::Koan
  def test_score_of_an_empty_list_is_zero
    assert_equal 0, score([])
  end

  def test_score_of_a_single_roll_of_5_is_50
    assert_equal 50, score([5])
  end

  def test_score_of_a_single_roll_of_1_is_100
    assert_equal 100, score([1])
  end

  def test_score_of_a_single_roll_of_1_is_100
    assert_equal 200, score([1,1])
  end

  def test_score_of_multiple_1s_and_5s_is_the_sum_of_individual_scores
    assert_equal 300, score([1,5,5,1])
  end

  def test_score_of_single_2s_3s_4s_and_6s_are_zero
    assert_equal 0, score([2,3,4,6])
  end

  def test_score_of_a_triple_1_is_1000
    assert_equal 1000, score([1,1,1])
  end

  def test_score_of_other_triples_is_100x
    assert_equal 200, score([2,2,2])
    assert_equal 300, score([3,3,3])
    assert_equal 400, score([4,4,4])
    assert_equal 500, score([5,5,5])
    assert_equal 600, score([6,6,6])
  end

  def test_score_of_mixed_is_sum
    assert_equal 250, score([2,5,2,2,3])
    assert_equal 550, score([5,5,5,5])
  end

  def test_score_of_a_triple_1_is_1000A
    assert_equal 1150, score([1,1,1,5,1])
  end

  def test_score_of_a_triple_1_is_1000B
    assert_equal 350, score([3,4,5,3,3])
  end

  def test_score_of_a_triple_1_is_1000C
    assert_equal 250, score([1,5,1,2,4])
  end
end

There are a few issues with the code:

Do not check for == nil when it is not specified as a valid value for the method. Here,checking for it and returning 0 might mask another problem.
Do not use return statements unless necessary. In ruby, almost everything is an expression, and methods return the value of the last expression. Here you can use if...elsif, or case instead of a series of if statement.
Do not modify parameters that come into your function (dice.sort!).
Do not use recursion if it makes the code less readable.

Here is a version of the code with the advice above applied:

def score(dice)
  score = 0
  counts = dice.each_with_object(Hash.new(0)) { |x, h| h[x] += 1 }
  (1..6).each do |i|
    if counts[i] >= 3
      score += (i == 1 ? 1000 : 100 * i)
      counts[i] = [counts[i] - 3, 0].max
    end
    score += counts[i] * (i == 1 ? 100 : 50)
  end
  score
end


TaskFactory.CompleteWhenBoth I'm trying to construct something which would work a bit like Zip for two tasks, but I'm a bit worried about race conditions in this code.

public static Task<R> ContinueWhenBoth<T1, T2, R>(this TaskFactory<R> factory, Task<T1> t1,
                                                 Task<T2> t2, Func<T1, T2, R> f)
{
    var result = new TaskCompletionSource<R>();

    t1.ContinueWith(t => { if(t2.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });
    t2.ContinueWith(t => { if(t1.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });

    return result.Task;
}

Is this a good way to go?

public static Task<R> ContinueWhenBoth<T1, T2, R>(this TaskFactory<R> factory, Task<T1> t1,
                                                 Task<T2> t2, Func<T1, T2, R> f)
{
    var result = new TaskCompletionSource<R>();

    int tasksLeft = 2;
    t1.ContinueWith(t =>
                    {
                    if(Interlocked.Decrement(ref tasksLeft) == 0 && t2.IsCompleted)
                        result.TrySetResult(f(t1.Result, t2.Result));
                    });
    t2.ContinueWith(t =>
                    {
                    if(Interlocked.Decrement(ref tasksLeft) == 0 && t1.IsCompleted)
                        result.TrySetResult(f(t1.Result, t2.Result));
                    });

    return result.Task;
}

You don't need to write this code yourself, there is already a method that does something very similar: Task.Factory.ContinueWhenAll().

ContinueWhenAll() doesn't directly support working with Tasks that return different types, but you can create a helper method that does:

public static Task<R> ContinueWhenBoth<T1, T2, R>(
    this TaskFactory<R> factory, Task<T1> t1, Task<T2> t2, Func<T1, T2, R> f)
{
    return factory.ContinueWhenAll(new Task[] { t1, t2 }, _ => f(t1.Result, t2.Result));
}

If you didn't want to use closure for performance reasons, you could use the array of Tasks and casting:

return factory.ContinueWhenAll(
    new Task[] { t1, t2 }, tasks => f(((Task<T1>)tasks[0]).Result, ((Task<T2>)t2).Result));

t1.ContinueWith(t => { if(t2.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });
t2.ContinueWith(t => { if(t1.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });

This code has a race condition: if t1 and t2 complete at the same time, then f might be executed twice. I don't think that's a good idea, since f might take a long time, or, worse, it might have side-effects.

if(Interlocked.Decrement(ref tasksLeft) == 0 && t2.IsCompleted)

The Decrement() here avoids the race condition from the previous code. But it also means that the check of t2.IsCompleted will always succeed, so you should remove it.

But your code also has a significant issue: if t1 or t2 fail with an exception, your resulting Task will never complete. (And on .Net 4.0, it will also crash the process some time in the future, because there is an uncaught exception in a Task.)


Length limited listagg for muliple m:n relations in a view I have a few views that all follow the same pattern. They select a primary key from some main table and then a few semicolon seperated strings of captions of related rows over many-to-many relations. All columns that are used in joins are either primary keys or have a manually set index and are of type NUMBER(18). The columns that hold the captions are either VARCHAR2 or - rarely - CLOB. Usually the view will be joined with the main table to select between 1 and 50 rows of the main table.

The creation of the semicolon seperated strings is done with the listagg function from Oracle 11gR2 because my research showed that this is the easiest and also most performant solution. There is just one problem: There could be so many related rows that the listagg function reaches the limit of 4000 chars of its VARCHAR2 result. I'm currently solving this by limiting the set that listagg operates on to 50 rows. This is just a random number that I've chosen, it should prevent errors and the users don't care for more than 50 results anyway. But of course, it's still ugly code.

If anyone has any ideas how to improve the following code, I'd be very happy. I'm especially interested in improvements that lead to less complexity, more performance or a better solution for the 4000 chars limit.

CREATE OR REPLACE VIEW MAILS_RELATED
(
  MAIL_ID,
  CUSTOMERS,
  PROJECTS,
  [some more columns]
)
AS
SELECT m.MAIL_ID,
       cust_list.CUSTOMERS,
       proj_list.PROJECTS,
       [some more columns]
  FROM MAILS m LEFT OUTER JOIN (
         SELECT inner.MAIL_ID,
                LISTAGG(inner.CUSTOMER_NAME, '; ')
                  WITHIN GROUP (ORDER BY inner.CUSTOMER_NAME) AS CUSTOMERS
           FROM (
                SELECT rel.MAIL_ID,
                       cust.CUSTOMER_NAME,
                       ROW_NUMBER() OVER
                         (PARTITION BY rel.MAIL_ID ORDER BY cust.CUST_ID) AS RN
                  FROM CUSTOMERS_MAILS rel INNER JOIN
                       CUSTOMERS cust ON (cust.CUST_ID = rel.CUST_ID)
                ) inner
          WHERE inner.RN <= 50
          GROUP BY inner.MAIL_ID
       ) cust_list ON (cust_list.MAIL_ID = m.MAIL_ID) LEFT OUTER JOIN (
         SELECT inner.MAIL_ID,
                LISTAGG(inner.PROJECT_NAME, '; ')
                  WITHIN GROUP (ORDER BY inner.PROJECT_NAME) AS PROJECTS
           FROM (
                SELECT rel.MAIL_ID,
                       proj.PROJECT_NAME,
                       ROW_NUMBER() OVER
                         (PARTITION BY rel.MAIL_ID ORDER BY proj.PROJ_ID) AS RN
                  FROM PROJECTS_MAILS rel INNER JOIN
                       PROJECTS proj ON (proj.PROJ_ID = rel.PROJ_ID)
                ) inner
          WHERE inner.RN <= 50
          GROUP BY inner.MAIL_ID
       ) proj_list ON (proj_list.MAIL_ID = m.MAIL_ID) LEFT OUTER JOIN
       [some more joins]

Oracle regular Character types are limited to 4000 bytes. Really, Oracle should find a way to extend that, but until they do, using CLOB is the only real option.

On the other hand, Oracle has a rich infrastructure available for creating user defined functions. You are already creating the view, you may as well extend that to include the function.

There are a number of articles out there that can help:

http://www.oracle-base.com/articles/misc/string-aggregation-techniques.php
http://www.oracle-developer.net/display.php?id=306
and others....

The one that is missing from those is the use of XML functions.... It is a relatively commonly used 'hack', to use the XML-processing functions in Oracle (and other databases) to reformat the data in a more convenient way, and then to extract from that XML just the parts you want.

So, ideally you should create a user-defined function for your problem, but, you can accomplish the same thing, perhaps faster, with a hack of XML....

Consider swapping the nested select:

     SELECT inner.MAIL_ID,
            LISTAGG(inner.CUSTOMER_NAME, '; ')
              WITHIN GROUP (ORDER BY inner.CUSTOMER_NAME) AS CUSTOMERS
       FROM (
            SELECT rel.MAIL_ID,
                   cust.CUSTOMER_NAME,
                   ROW_NUMBER() OVER
                     (PARTITION BY rel.MAIL_ID ORDER BY cust.CUST_ID) AS RN
              FROM CUSTOMERS_MAILS rel INNER JOIN
                   CUSTOMERS cust ON (cust.CUST_ID = rel.CUST_ID)
            ) inner
      WHERE inner.RN <= 50
      GROUP BY inner.MAIL_ID

with:

Select CM.MAIL_ID,
  substr(xmlcast(
    xmlagg(
      xmlelement(E, '; ' || C.CUSTOMER_NAME)
      ORDER BY C.CUSTOMER_NAME
    ) AS CLOB -- or AS VARCHAR2(4000)
  ), 3
) as CUSTOMERS
from CUSTOMERS C, CUSTOMER_MAIL CM
where C.CUST_ID = CM.CUST_ID
group by CM.MAIL_ID

I have put together a simple SQLFiddle that shows this select in action. It assumes two sets of users, (some male names, some female names), and it puts the different users in to different MAIL_ID's.

Once you have swapped out these nested statements, it will significantly shorten your query.... Your handling code though will have to change. Getting a CLOB back from the view is a big change.... and it is best to handle that in a different way....

Edit: About this operation as a general problem

At an academic level, the problem you are experiencing is because you are doing a non-relational operation in a relational database. Data in a relational database is treated using set arithmetic. You have sets of data that you can intersect, union, filter, and otherwise manipulate. The problem you are tying to solve in this question is the conversion of a set of values in to a single merged value (with internal consistency requirements). This type of operation is not well defined in set theory.

The standard/recommended mechanism for solving this problem is to export the data set in to an external application, and process the data from there. Any system you use in an SQL query to remove the set-like nature of the data is a 'hack', and that is why you are unsatisfied with the result you are seeing. The right solution is to not use SQL.


Adjusting integer based on multiple elements in int array I'm working with the following code which adjusts an int based on multiple elements of an integer array. I am wondering if there's a cleaner, easier-to-read construct for this:

int Blue = 0;
int[] Adjustments = new int[24];
// ... populate Adjustments

if (Adjustments[0] == 255)
    Blue = 128;
else if (Adjustments[0] == 0)
    Blue = 0;

if (Adjustments[1] == 255)
    Blue += 64;
else if (Adjustments[1] == 0)
    Blue += 0;

if (Adjustments[2] == 255)
    Blue += 32;
else if (Adjustments[2] == 0)
    Blue += 0;

if (Adjustments[3] == 255)
    Blue += 16;
else if (Adjustments[3] == 0)
    Blue += 0;

if (Adjustments[4] == 255)
    Blue += 8;
else if (Adjustments[4] == 0)
    Blue += 0;

if (Adjustments[5] == 255)
    Blue += 4;
else if (Adjustments[5] == 0)
    Blue += 0;

if (Adjustments[6] == 255)
    Blue += 2;
else if (Adjustments[6] == 0)
    Blue += 0;

if (Adjustments[7] == 255)
    Blue += 1;
else if (Adjustments[7] == 0)
    Blue += 0;

Any suggestions?

Edit

Here is the revised version based on suggestions provided:

Blue = 0;
Green = 0;
Red = 0;
int[] Adjustments = new int[24];
// ... populate Adjustments

int[] AdjustmentStops = new[] { 128, 64, 32, 16, 8, 4, 2, 1 };
for (int i = 0; i < 8; i++)
{
    Blue += Adjustments[i] == 255 ? AdjustmentStops[i] : 0;
    Green += Adjustments[i + 8] == 255 ? AdjustmentStops[i] : 0;
    Red += Adjustments[i + 12] == 255 ? AdjustmentStops[i] : 0;
}

At the risk of being 'clever':

int Blue = 0;
int[] Adjustments = new int[24];
// ... populate Adjustments

for (int blueIndex=0; blueIndex<8; blueIndex++)
{
   if (Adjustments[blueIndex] == 255)
   {
      Blue |= (1 << (7 - blueIndex));
   }
}

Since in effect you're setting bits anyway I think using bitwise operations is idiomatic here.


rxjava observables and scala futures In a simple webapp I have been using as a playground, I am playing with using rxjava to perform a number of operations in a reactive manner.  I have been using reactivemongo to connect to my Mongo database, but I find that for anything more than a trivial CRUD operation I often end up trying to reduce a Future[List[Future[T]]] into a Future[List[T]].  This can happen multiple times in the execution chain, before I finally hand it back to the Play framework.

I am playing with rxjava to try to convert these reactive operations into Observables, so that I can combine them using rxjava's utility methods, but I was wondering if anyone had already solved this problem.

Here is an example from my most complex action:

def import = Action {
   //scan the base folder for app IDs (Future[List[UUID]])
   val summaries = ftpWrapper.scanUserIdsAsync

   //import all the files in each of those folders (Future[List[LogImportResult]])
   val processResults = summaries.flatMap(lst => {

     //List[Future[List[LogImportResult]]]
     val imports = lst.map(user => logProcessor.importLogFolderAsync(user))

     //fold together the several lists of insertion results into one list
     Future.fold(imports)( List[LogImportResult]() )( _ ++ _ )
   })

   Async {
     //display the insertion results, or throw.
     processResults.transform(r => Ok(views.html.logImportResult(r)), ex => throw ex)
   }
}

The above code calls into my services to do the following:

Locate all folders on an FTP server whose names are UUIDs
Import all files inside each folder into the Mongo database:

check if the file already exists in mongo
if no, then insert the data from the file as a BSONDocument

combine the results of importing each folder into one giant list
display the results in the view

Most of these actions are non-blocking asynchronous, and as such return a future.  I have code all over the place in this app to combine these futures together, so I am interested in seeing if rxjava can simplify that.  I have already been working on some ideas here: https://gist.github.com/gburgett/6106724

So, has anyone encountered something like this?  If so, do you have a better way to go about it than simply combining futures like this?

It could be the case that using Scala.Rx's reactive variables simplify your use case since they propagate change state as they happen. In particular, chaining those smart Var's together might help you simplify the task. Please have a look at:

https://github.com/lihaoyi/scala.rx

There you also find a link to the underlying publication that elaborates on the concept further.


Regex pattern definition implementation My requirement is to design a part of a solution in such matter that specific placeholders are replaces dynamically with a defined logic and new rules can be easily added.

Simple example:

  Today is {today}. The year is {year}. Last year was {year-1}.

Should output something like:

  Today is 26 Jan 2018. The year is 2018. Last year was 2017.

There are several more business and context specific placeholders and new rules can follow frequently.

As such I came up with this class:

public class PlaceholderDefinition
{
    public string Name { get; }
    public string Pattern { get; }
    private readonly Func<Match, string> _logic;

    public PlaceholderDefinition(string name, string pattern, Func<Match, string> logic)
    {
        Name = name;
        Pattern = pattern;
        _logic = logic;
    }

    public string Apply(Match match) => _logic.Invoke(match);
}

To fulfill the above example, I register the following PlaceholderDefinitions in the service configuration:

RegisteredPlaceholders.Add(
    new PlaceholderDefinition(
        "Today",
        @"{[tT][oO][dD][aA][yY]}",
        (_) => DateTime.Today.ToShortDateString()
    ));

RegisteredPlaceholders.Add(
    new PlaceholderDefinition(
        "Year",
        @"{[yY][eE][aA][rR]}",
        (_) => DateTime.Today.Year.ToString()
    ));

RegisteredPlaceholders.Add(
    new PlaceholderDefinition(
        "YearAddition",
        @"(?:{[yY][eE][aA][rR])([+-])(\d+)}",
        (m) =>
        {
            var operation = m.Groups[1].Value;
            int.TryParse(m.Groups[2].Value, out int value);
            return (operation == "+" ? DateTime.Today.Year + value : DateTime.Today.Year - value).ToString();
        }
    ));

The RegisteredPlaceholders is an enumerable of PlaceholderDefinition held in the container.

The logic of these patterns/placeholders is applied like this:

foreach (var placeholder in RegisteredPlaceholders)
 {
     var match = new Regex(placeholder.Pattern).Match(content);
     while (match.Success)
     {
         content = content.Remove(match.Index, match.Length).Insert(match.Index, placeholder.Apply(match));
         match = match.NextMatch();
     }
 }

I decided to take this approach instead of using Regex.Replace() to not reveal the content or text to the class defining the pattern logic. Maybe there is a more elegant solution, that I didn't come up with.

Every critique, improvement proposal, code smells are welcome.

Try to use named group. As your pattern becomes more complex, it will get harder to maintain.

@"(?:{[yY][eE][aA][rR])([+-])(\d+)}"
var operation = m.Groups[1].Value;
int.TryParse(m.Groups[2].Value, out int value);

PlaceholderDefinition implementation requires way too much boilerplate code to use:

 foreach (var placeholder in RegisteredPlaceholders)
  {
      var match = new Regex(placeholder.Pattern).Match(content);
      while (match.Success)
      {
          content = content.Remove(match.Index, match.Length).Insert(match.Index, placeholder.Apply(match));
          match = match.NextMatch();
      }
  }

It could be boiled down to: content = placeholder.Apply(text);
There is no need to use Match.NextMatch & string.Remove, when you can use Regex.Replace
_logic seems to be a poorly chosen name. While it describes the essence, but not its concrete job. You should name Func with "Selector, factory, builder, ...", and prefix it with a noun ("result, replacement, ...") and the private member prefix _ (if this is the convention, you choose to follow).

Modified PlaceholderDefinition class:

public class PlaceholderDefinition
{
    public string Name { get; }
    public Regex Pattern { get; }

    private readonly Func<GroupCollection, object> _replacementSelector;

    public PlaceholderDefinition(string name, string pattern, Func<GroupCollection, object> replacementSelector)
         : this(name, new Regex(pattern), replacementSelector)
    {
    }
    public PlaceholderDefinition(string name, string pattern, RegexOptions options, Func<GroupCollection, object> replacementSelector)
         : this(name, new Regex(pattern, options), replacementSelector)
    {
    }
    public PlaceholderDefinition(string name, Regex pattern, Func<GroupCollection, object> replacementSelector)
    {
        this.Name = name;
        this.Pattern = pattern;
        this._replacementSelector = replacementSelector;
    }

    public string Apply(string input) => Pattern.Replace(input, m => _replacementSelector(m.Groups).ToString());
}

There is various overloads of ctor to accommodate different needs, like the one suggested by @t3chb0t. Feels free to add more, if it help you to keep the declarations as clean as possible.

And, the rest of the code:

var text = "Today is {today}. The year is {year}. Last year was {year-1}.";
var placeholders = new List<PlaceholderDefinition>();
placeholders.Add(new PlaceholderDefinition("Today", @"{today}", RegexOptions.IgnoreCase, _ => DateTime.Today.ToShortDateString()));
placeholders.Add(new PlaceholderDefinition("Year", @"{year}", RegexOptions.IgnoreCase, _ => DateTime.Today.Year));
placeholders.Add(new PlaceholderDefinition(
    "YearAddition",
    @"{year(?<sign>[+-])(?<value>\d+)}",
    RegexOptions.IgnoreCase,
    g => DateTime.Today.Year + int.Parse(g["sign"].Value + g["value"].Value)
));

foreach (var placeholder in placeholders)
{
    text = placeholder.Apply(text);
}

The _replacementSelector takes in GroupCollection, so we can skip writing m => m.Group..., and the object TResult allows us to skip the necessary .ToString() or a parenthesis on binary operation.
int.Parse will parse number prefixed by a + sign as positive number, allowing us to skip the ternary operation.


Calculate the largest palindromic number from the product of two 6-digit numbers (100000 to 999999) Are there any efficient ways to solve this problem,
for example using bitwise operator?

    public static boolean isPal(long num)
    {
        String numStr = Long.toString(num);
        String rNumStr = "";

        boolean result = false;

        for (int i = numStr.length() - 1; i >= 0 ; --i)
            rNumStr += numStr.charAt(i);

        //System.out.println(numStr + "," + rNumStr);
        try
        {
            if (Long.parseLong(numStr) == Long.parseLong(rNumStr))
                result = true;
            else
                result = false;
        }catch (NumberFormatException e) {
            //System.err.println("Unable to format. " + e);
        }
        return result;

    }

    public static void calcPal(int rangeMin, int rangeMax)
    {
        long maxp = 0, maxq = 0, maxP = 0;
        for (int p = rangeMax; p > rangeMin; p--)
            for (int q = rangeMax; q > rangeMin; q--)
            {
                long P = p * q;
                if (isPal(P))
                    if (P > maxP)
                    {
                        maxp = p; maxq = q; maxP = P;
                    }
            }
        System.out.println(maxp + "*" + maxq + "=" + maxP);
    }

    public static void main(String[] args)
    {
        calcPal(10, 99);
        calcPal(100, 999);
        calcPal(9000, 9999);
        calcPal(10000, 99999);
        calcPal(990000, 999999);
    }

The largest palindrome which can be made from the product of two 2-digit (10 to 99) numbers is 9009 (91 × 99). Write a function to calculate the largest palindromic number from the product of two 6-digit numbers (100000 to 999999).

In an interview setting it is quite hard to come with an efficient solution (unless you happen to be very good in mental multiplication; however a \$99 * 91\$ example is a strong hint). The key to an efficient solution is an observation that

\$999999 * 999001 = 999000000999\$

is a quite large palindromic product. It means that you don't have to test the entire 6-digit ranges of multiplicands. It is enough to test multiplicands only in \$[999001.. 999999]\$ range. Just \$10^6\$ candidate pairs rather than \$10^{12}\$.

BTW, a similar identity holds for products of longer numbers as well.

Next, you may notice that there are just one thousand palindromic numbers larger than \$999000000999\$ (they are in form of 999abccba999), and to qualify as a solution is must have a 6-digit factor larger than \$999001\$. This implies the following algorithm (in pseudocode):

base = 999000000999
abc = 999
while abc >= 0
    cba = reverse_digits(abc)
    number = base + abc * 1000000 + cba * 1000
    for factor in 999001 to sqrt(number)
        if number % factor == 0:
           return number
    abc -= 1

The reverse_digits of a 3-digit number could be done extremely fast (a lookup table, for example). Still a \$10^6\$ or so rounds, but no expensive tests for palindromicity.

All that said, since the problem stems from Project Euler #4 it is possible that it admits a more elegant number-theoretical solution.


Calculate age given the birth date in the format YYYYMMDD Here is method to calculate age given the birth date in the format YYYYMMDD.
function getAge(dateString) {
    var today = new Date();
    var birthDate = new Date(dateString);
    var age = today.getFullYear() - birthDate.getFullYear();
    var m = today.getMonth() - birthDate.getMonth();
    if (m < 0 || (m === 0 && today.getDate() < birthDate.getDate())) {
        age--;
    }
    return age;
}

Is it possible to make it more readable and easy to understand?
I don't want moment.js

Non standard date string YYYYMMDD
As pointed out in the comments "Looks quite reasonable..." and as such there is not too much to review...
Well apart from the fact that new Date("19671031") is not a valid date The date string as parsed by Date must be a valid date strings. (I did not spot this until halfway through the review)
Ignoring the fact your code does not work as a simple oversight and continuing on as initially started...
...I will therefor nit pick, and I do stress the point this is just nit picking for sake of a review.
Constants
Constants as const The named vars today, birthDate, m can all be const. In fact the whole thing can be rewritten to only use constants.
Names
Names are too verbose. The general rule is to use the scope to infer meaning, for names to preference 1 word over 2, 2 words over 3, etc... Short words rather than long. Common abbreviations instead of full words.

birthDate can be birth as date is inferred
dateString can be dateStr using the common abbreviation for string
today could be now. In context today is the most accurate however nothing is lost if now is used.
m does not work as month. The value represents a quantitative query (How many months?) and as such months or better yet the abbreviated mths... eww ... drop the s for mth

Rewrite A
And then I realize it does not work
function getAge(dateStr) {
    const birth = new Date(dateStr);
    const now = new Date();
    const mth = now.getMonth() - birth.getMonth();
    const adjust = mth < 0 || (mth === 0 && now.getDate() < birth.getDate()) ? 1 : 0;
    return now.getFullYear() - birth.getFullYear() - adjust;
}

Responsibility
A general rule for functions is that they should only do one thing. Your function has two tasks, parse the date string and calculate the age.
As the date string is likely an un-trusted source (eg user input) and as such requires vetting which is outside the functions responsibility (inferred by the name getAge) the function should accept a Date rather than a string to parse.
This solves the problem of incorrectly formatted date string.
Rewrite B
As the birth date is now an argument the simple name birth does not hold enough meaning to be clear so it is changed back to birthDate
function getAge(birthDate) {
    const now = new Date();
    const mth = now.getMonth() - birthDate.getMonth();
    const adjust = mth < 0 || (mth === 0 && now.getDate() < birthDate.getDate()) ? 1 : 0;
    return now.getFullYear() - birthDate.getFullYear() - adjust;
}

Parsing and vetting
The task of vetting and parsing the date string can now be handed to a more appropriate function.
The function assumes that the format is YYYYMMDD and returns a date only if the Date accepts the year, month, and day extracted from the string, that the date is earlier than now. It will return an age that fits within the Date object range (age of historical births)
If the date is invalid it can be said to be undefined and as such the function returns a Date or undefined.
function parseBirthDate(dateStr) {
    const birth = new Date(dateStr.slice(0,4), dateStr.slice(4,6), dateStr.slice(6,8)););
    const since = Date.now() - birth.getTime();
    return isNaN(since) || since < 0 ? undefined : birth;
}

Using the two functions to get an age.
const birth = parseBirthDate("19671031");
const age = birth ? getAge(birth) : undefined;

Testing
A simple manual test snippet accepts a string input

function getAge(birthDate) {
    const now = new Date();
    const mth = now.getMonth() - birthDate.getMonth();
    const adjust = mth < 0 || (mth === 0 && now.getDate() < birthDate.getDate()) ? 1 : 0;
    return now.getFullYear() - birthDate.getFullYear() - adjust;
}
function parseBirthDate(dateStr) {
    const birth = new Date(dateStr.slice(0,4), dateStr.slice(4,6), dateStr.slice(6,8));
    const since = Date.now() - birth.getTime();
    return isNaN(since) || since < 0 ? undefined : birth;
}
birthDate.addEventListener("change", () => {
    const birth = parseBirthDate(birthDate.value);
    const age = birth ? getAge(birth) : undefined;
    ageDisplay.textContent = age !== undefined ?
        "Age: " + age + " years old" :
        "Invalid birth date!";
})

Enter birth date:
<input type="text" id="birthDate"  maxlength="8" placeholder = "YYYYMMDD" size="8">


Implementation of insertion sort Information about my code:

I am following this MIT OCW algorithms course. The first lecture described insertion sort and merge sort. I implemented insertion sort in C.

The algorithm is structured as a function called from the main function. The array to be sorted is allocated dynamically in the main function but can also be statically allocated.

What I am looking for:

I am looking whether the code can be optimized without changing the algorithm (insertion sort), whether it follows the best-practices of programming in C and does it have proper readability factor?

#include<stdio.h>
#include<stdlib.h>

void insertion_sort(int arr[], int length)
{
    /*
    Sorts into non-decreasing order
    To change the sorting to non-increasing order just change
    the comparison in while loop
    */
    register int j, k;
    int temp;

    for(j = 1; j < length; j++)
    {
        temp = arr[j];
        k = j - 1;
        while (k >= 0 && arr[k] > temp)
        {
            arr[k + 1] = arr[k];
            k--;
        }
        arr[k + 1] = temp;
    }
}

int main()
{
    int length;
    register int i;
    int *arr;

    //Finds the length of array and dynamically creates array of that length
    scanf("%d", &length);
    if ( (arr =  (int *)malloc(sizeof(int) * length)) == NULL)
    {
        printf("Not enough memory");
        return 1;
    }

    //Reads the array
    for(i = 0; i < length; i++)
        scanf("%d", &arr[i]);

    //Calls the sorting algorithm
    insertion_sort(arr, length);

    //Prints the sorted array
    for(i = 0; i < length; i++)
        printf("%d ", arr[i]);

    //Frees the memory allocated and returns
    free(arr);
    return 0;
}

I cannot offer any optimization, only a few comments on best practice (as I
see it). There's very little to say; the code is nice.

Use of register is unnecessary (and hence rare).  The compiler may well
ignore it and it probably has a better idea of what to put in registers than
you or I.
It is best to define variables nearer to where they are first used.  This
reduces their scope and makes for easier reading. For example in
insertion_sort I would do:

for(int j = 1; j < length; j++)
{
    int temp = arr[j];
    int k = j - 1;
    while (k >= 0 && arr[k] > temp)
    ...

So I defined the loop variable j in the loop and temp and k at the
point of first use.  (This makes even more sense in C++ where an early definition incurs the overhead of a default construtor call.)
Your while-loop could arguably be a for-loop.  A for-loop makes the loop
conditions more immediately obvious, but in such a short loop it makes no
difference.
In main you cast the return from malloc.  This is necessary in C++ but
not in C.  And on an error in system and library calls that set the global
errno, such as malloc, I would use perror("malloc") which will print a
relevant error message to stderr, instead of rolling your own with printf (which prints to stdout).
Again your for-loops in main would be better defining loop variable i
in the loop.  Also these loops should have braces:

for (int i = 0; i < length; i++) {
    scanf("%d", &arr[i]);
}

Strictly speaking braces are not necessary but they are considered good
practice, as they prevent a class of error such as someone adding a printf:

for (int i = 0; i < length; i++)
    scanf("%d", &arr[i]);
    print("%d", &arr[i]);

For safety, you should always check user input before using it, such as
length here.
Your comments in main are really just stating the obvious and as such
could be considered 'noise'.  The comment in insertion_sort on the other
hand is more useful, although 'non-decreasing' and 'non-increasing' are odd
ways to say 'increasing' and 'decreasing' respectively.
Really nit-picking, I prefer a space after for and while and no space
between brackets in the expression if ((arr = malloc(sizeof...


jQuery element reloader I have looked around for an already existing script for this, but couldn't find one, so i decided to make my own.

The point of this is to make it easy to reload an element, whether its a form or a link.

To use the script, you add a class to the link/form called "js_container-reloader".
What the script does is it simply gets the content of the action url or href, creates a wrapper around the element you are reloading, takes the ajax response and puts it in to the wrapper, then removes the wrapper.

Its working as it should at the moment, but i wonder if there is a better way of doing this.

//Form element reloader
$(document).on("submit", ".js_container-reloader", function(e){
    e.preventDefault();

    //Call the reloader function
    elementReloader(e.currentTarget);
});

//Link element reloader
$(document).on("click", ".js_container-reloader", function(e){
    e.preventDefault();

    //Call the reloader function
    elementReloader(e.currentTarget);
});

/*  Element reloader function
*   Takes an element and the type of request this is */
function elementReloader(element){

    //Set variables
    var element = $(element),
        requestUrl,
        requestType,
        data,
        reloadContainer;

    //Wrap the element in a reloader div and save the div to a variable
    element.wrap("

");
    reloadContainer = element.closest('.js_reload-container');

    //If the element is a form
    if(element.is("form")){

        //Save the forms action
        requestUrl = element.attr('action');

        //Save the request type
        requestType = element.attr('method');

        //Save the forms data
        data = element.serialize();

    //If the request should be done by GET
    }else{

        //Get the forms action
        requestUrl = element.attr('href');

        //Save the request type
        requestType = "GET";
    }

    //Run the ajax script
    $.ajax({
        type: requestType,
        url: requestUrl,
        data: data,
        success: function(result){

            //If the result returned false, reload the alert container
            if(!result){
                reloadAlertContainer();

            //If its not false then load the container with the result
            }else{

                //Add the result to the wrapper
                reloadContainer.html(result);

                //Remvoe the wrapper
                reloadContainer.find(".js_container-reloader").unwrap();
            }
        },
        error: function(xhr) {

            /*  Reload the alerts container
            *   An alert is saved to the session if there was an error
            *   with any script that was loaded.
            *   This function reloads the container which shows the session alert. */
            reloadAlertContainer();
        },
    });
}

I don't assume my code is expert level as i am not an expert, but the goal is to make it as easy and dynamic as possible for future implementations.
I tried looking in to combining the code, especially the first 2 functions for the event listeners, together in to one function, but couldn't find a way that seemed like a proper solution.

Just to clarify about the form posting - i do the form validation and checking in php, in the actual file that is being loaded, and not in JavaScript/jQuery. This script is just for reloading elements.

Sorry I was busier earlier. Here is a more complete review. Just a few things off the top of my head.

Try and be consistent with your class naming. Instead of  .js_container-reloader and js_reload-container use a common prefix, i.e. .js_reloader-trigger (or even just .js_reloader) and .js_reloader-container
You can use a single call to the on function like:

$(document).on("submit", ".js_container-reloader", ...
var element = $(element), is syntactically incorrect since element is already defined as a parameter. One options is just use element = $(element);  another pattern I often see is using a $ prefix on variables to indicate jQuery objects, i.e. var $element = $(element),
requestUrl = element.attr('action'); would be better asrequestUrl = element.prop('action'); the latter will retrieve the default method for the form if no attribute is set.
I personally think the use of wrap andunwrap` is an odd way to handle adding an element. I would remove the wrap and container code and just have something like the following on success:

success: function(result){
   ....
   element.after(result);
   element.remove();

I would add a .js_reloader-loading class to the element before starting the ajax query so that you can style it differently (fade it, add a spinner, etc).
Lastly you probably want to implement some kind of logic to prevent multiple attempts at loading the form.


Basic LinkedList implementation Please bear in mind, I've only started learning c++ this week, and as such, would love topic suggestions for which I should master, to become better at C++.

Below, is my code for a singly linkedlist. It doesn't have complete functionality, just the ability to print, append, and add a node at a specified index.

Currently I'm aware that each node is created on the Heap. I have no idea how to delete the nodes in a LinkedList once the LinkedList itself is deleted. I'm also not sure how to create a linkedlist without pointers.

#include<iostream>

template <typename T>
struct Node {
    Node(T value);
    T data;
    Node<T> *next;
};

template <typename T>
Node<T>::Node(T value){
    this->data = value;
    this->next = nullptr;
}

template <typename T>
class LinkedList {
public:
    Node<T> *head;
    LinkedList<T>();
    void push_back(T data);
    void push_back(int index, T data);
    void print();
};

template <typename T>
LinkedList<T>::LinkedList(){
    this->head = nullptr;
}

template <typename T>
void LinkedList<T>::push_back(T data){
    Node<T> *n = new Node<T>(data);
    if (this->head == nullptr){
        this->head = n;
    } else {
        Node<T> *cursor = this->head;
        while(cursor->next != nullptr){
            cursor = cursor->next;
        }
        cursor->next = n;
    }
}

template <typename T>
void LinkedList<T>::push_back(int index, T data){
    Node<T> *cursor = this->head;
    unsigned const position = index - 1;
    int i = 0;
    while (i < position && cursor->next != nullptr){
        cursor = cursor->next;
        i++;
    }
    Node<T> *nodeToInsert = new Node<T>(data);
    Node<T> *temp = cursor->next;
    cursor->next = nodeToInsert;
    nodeToInsert->next = temp;
}

template <typename T>
void LinkedList<T>::print(){
    Node<T> *cursor = this->head;
    while(cursor != nullptr){
        std::cout << cursor->data << '\n';
        cursor = cursor->next;
    }
}

int main(){

    LinkedList<int> intL;
    intL.push_back(1);
    intL.push_back(2);
    intL.push_back(3);
    intL.print();

    LinkedList<std::string> sL;
    sL.push_back("one");
    sL.push_back("two");
    sL.push_back("three");
    sL.push_back(2, "two point five");
    sL.print();
}

Also, should I potentially add an abstract class/or a c++ version of an interface for my LinkedList to follow, and then I could use this same abstraction for, say, a stack?

Notes

Currently I'm aware that each node is created on the Heap.

No such thing as heap in C++ (or stack technically).
You mean dynamically allocated memory in the situation.
Note: Though the language does not specify the need for a stack most implementations will use one. But the concept itself is not useful for thinking about C++ objects. You should look up "Automatic/Static/Dynamic" storage duration.

I have no idea how to delete the nodes in a LinkedList once the LinkedList itself is deleted.

If you create something with new ou delete it with delete. I'll cover that below. When an object is destroyed (either manually or by going out of scope) its destructor will always be called.

I'm also not sure how to create a linkedlist without pointers.

I don't think I could do that either.

Also, should I potentially add an abstract class/or a c++ version of an interface for my LinkedList to follow.

In C++ we use this thing called "Concepts". A linked list is an implementation of the "Concept: Container" there is no interface for a concept but rather a standard set of things that it should implement. https://en.wikipedia.org/wiki/Concepts_(C%2B%2B)
Thus allows for template to do "Duck" typing.
If it quacks like a duck then it can be used as a duck.
Basically templates allow you to use any type as long as the function you use are available for that type and it compiles it is said to follow the duck typing interface of the template.

and then I could use this same abstraction for, say, a stack?

Now there is already a type std::stack that underneath uses an object that implements the Concept of a SequenceContainer.
And defines these functions (As defined for a Sequenced Container).

back()
push_back()
pop_back()

See: https://en.cppreference.com/w/cpp/named_req/SequenceContainer
Code Review
Don't need to mention the template type inside the templates object.
template <typename T>
struct Node {
    Node(T value);
    T data;
    Node<T> *next;  // That <T> is not needed here.
                    // The compiler assumes its a T unless you
                    // manually specify otherwise.
};

The * and & and && are part of the type infomration. So in C++ (unlike C) they are usually placed next to the type information not the member.
    Node*   next;   // Next has a type Node* (or Node Pointer).

Don't use this->
template <typename T>
Node<T>::Node(T value){
    this->data = value;
    this->next = nullptr;
}

The only time you need to use this-> is when you have a local variable that shadows a member variable. If you have a shadowing problem them the compiler can not tell when you accidently leave off the this-> and thus shadowing leads to hard to detect bugs.
So it is better to use unique names and get the compiler to warn you when you have a shodowed variable. Then your naming of variables becomes better and you don't accidently use the wrong variable.

For short function put them in the class definition.
template <typename T>
Node<T>::Node(T value){
    this->data = value;
    this->next = nullptr;
}

Thats short and because you don't need the template <typename T> in the class it becomes even shorter.

Prefer to use the initializer list over constructor code.
When an object is created all its members are initialized in the order declared in the class. If there are no members initialized in the initializer list they are zero or default initialized depending on the exact declaration.
But if you are setting the values in the constructor code this means you are re-setting them in the code. For complex objects this means you are calling the operator= to redo work that was done in the initializer list.
template <typename T>
Node<T>::Node(T value)
    : data()            // You have an implicit initialization here
    , next()            // even if you don't do it yourself.
{
    data = value;       // If the above initialization did work
    next = nullptr;     // Then these two assignments become extra work.
}

Your copy constructor passes the data by value:
Node<T>::Node(T value)     // Pass by value causes a copy of value here.
    : data()               // Data is default constructed here.
                           // See above.
{
    this->data = value;    // Copy assignment causes another copy to be
                           // made here.

If you are keeping the data you need to copy it at some point but you don't need to copy it twice. So pass by const reference to avoid a copy.
Node<T>::Node(T const & value)  // Pass by reference to avoid copy.
    : data(value)               // Construct data using the copy constructor.
{

In modern C++ we have the concept of move semantics. This allows an object to move expensive parts to another object if it is not going to be used again. Think of passing a huge array into an object. You may not want to copy it (as above). If you are not going to use the data again you want to move the object.
Node<T>::Node(T&& value)        // The double && indicates it will bind
                                // To an r-value reference (something that can be moved)
    : data(std::move(value))    // You pass the T object to the move
                                // constructor of the type T to allow it to
                                // move itself.
{

Better declaration for Node:
template <typename T>
struct Node
{
    Node(T const& value)
        : data(value)
        , next(nullptr)
    {}
    Node(T&& value)
        : data(std::move(value))
        , next(nullptr)
    {}
    T        data;
    Node*    next;
};

But even better news. Since the above is such a common pattern (Adding constructors and assignment operators like this). That this is done automatically for all types (unless you start manually defining them). So the above type can be defined simply by doing:
template <typename T>
struct Node
{
    T        data;
    Node*    next;
};

Note: Normally this is a bad thing if your object contains a pointer. BUT if we consider the Node as a non owning of the pointer it works. But you have to move the responsibility for owning onto the LinkedList class. So we will cover that below.

LinkedList is a class that contains an "OWNED" pointer (unlike Node). So this class you do need to specify a couple of methods or things go wrong.
Look up the rule of three.
If you don't specify them the compiler will automatically generate:

CopyConstructor LinkedList(LinkedList const&);
CopyAssignment  LinkedList& operator=(LinkedList const&);
Destructor      ~LinkedList()

The constructors do a shallow copy and the destructor simply makes sure that all members are correctly destroyed. So in this case they do:
 LinkedList(LinkedList const& copy)
     : head(copy.head)
 {}
 LinkedList& operator=(LinkedList const& copy)
 {
     head = copy.head;
     return *this;
 }
 ~LinkedList()
 {
     /* Nothing is done for pointers */
 }

So what you say. This does not affect me. This is true because currently you have not defined the destructor. But if you correctly define the destructor the constructors will cause problems.
 ~LinkedList()
 {
     deleteTheListPointedAtBy(head);  // This is what you should do.
 }

This causes problems because the above 2 constructors allow:
 {
      LinkedList<int>  a;
      a.push_back(1);
      LinkedList<int>  b(a);   // We are allowed to copy a like this.
                               // But both a and b have a head that points
                               // at the same value.

 } // At this point both a and b are destroyed.
   // Thus there destructors are called.
   // Thus they both try and delete all the same nodes.
   // which will cause problems.

As above in Node. You should think about move semantics. There are two methods added in C++11 that change the rule of three into the rule of 5.

MoveConstructor LinkedList(LinkedList&&);
MoveAssignment  LinkedList& operator=(LinkedList&&);

As above use the initializer list:
template <typename T>
LinkedList<T>::LinkedList(){
    this->head = nullptr;
}

As above pass by const reference to avoid a copy.
template <typename T>
void LinkedList<T>::push_back(T data){

Same again pass by const reference:
template <typename T>
void LinkedList<T>::push_back(int index, T data){

This is good and looks correct.
    Node<T> *cursor = this->head;
    unsigned const position = index - 1;
    int i = 0;
    while (i < position && cursor->next != nullptr){
        cursor = cursor->next;
        i++;
    }
    Node<T> *nodeToInsert = new Node<T>(data);
    Node<T> *temp = cursor->next;
    cursor->next = nodeToInsert;
    nodeToInsert->next = temp;

But we could simplify it a bit:
    Node* cursor = head;
    for(int loop = 0;loop < position && cursor->next !- nullptr;++loop,cursor = cursor->next)
    {}

    cursor->next = new Node<T>(data, cursor->next);

This also works well.
template <typename T>
void LinkedList<T>::print(){
    Node<T> *cursor = this->head;
    while(cursor != nullptr){
        std::cout << cursor->data << '\n';
        cursor = cursor->next;
    }
}

But you assume the output is std::cout. I would pass the output stream as a parameter (it can default to std::cout`). This makes the print much more flexable.
The print() method does not change the state of the object so it should be marked const.
In C++ we normally use operator<< to output stuff. So you may want to write that function so it looks more natural. It can simply call the print method.
template <typename T>
void LinkedList<T>::print(std::ostream& str) const
{
    Node<T> *cursor = this->head;
    while(cursor != nullptr){
        str << cursor->data << '\n';
        cursor = cursor->next;
    }
}
template<typename T>
std::ostream& operator<<(std::ostream& str, LinkedList<T> const& list)
{
    list.print(str);
    return str;
}

Your missing destructor:
template<typename T>
~LinkedList()
{
    Node<T>*   old;
    while (head) {
        old = head;
        head = head->next;
        delete old;
    }
 }

Alternative Implementation:
template <typename T>
class LinkedList
{
    // Make Node a private member of the Linked List.
    struct Node
    {
        T      data;
        Node*  next;
    };
    Node*      head;
    Node*      tail;

    public:
        ~LinkedList() {deleteList();}

        LinkedList()
            : head(nullptr)
            , tail(nullptr)
        {}

        LinkedList(LinkedList const& copy)
            : LinkedList()
        {
            for(Node* other = copy->head; head; head = head->next) {
                push_back(other->data);
            }
        }
        LinkedList operator=(LinkedList const& copy)
        {
            LinkedList   tmp(copy);  // See Copy and Swap Idiom
            swap(tmp);
            return *this;
        }

        LinkedList(LinkedList&& move) noexcept
            : LinkedList()
        {
            swap(move);
        }
        LinkedList operator=(LinkedList&& move) noexcept
        {
            swap(move);
            return *this;
        }

        void swap(LinkedList& other) noexcept
        {
            using std::swap;
            swap(head, other.head);
            swap(tail, other.tail);
        }
        friend void swap(LinkedList& lhs, LinkedList& rhs) noexcept
        {
            lhs.swap(rhs);
        }

        void push_back(T const& data)
        {
            pushbackNode(new Node{data, nullptr});
        }
        void push_back(T&& data)
        {
            pushbackNode(new Node{std::move(data), nullptr});
        }

        void push_back(int index, T const& data);
        void push_back(int index, T&& data);

        void print() const;
        friend std::ostream& operator<<(std::ostream& str, LinkedList const& data) {
            data.print(str);
            return str;
        }
    private:
        void deleteList()
        {
            Node*  old;
            while (head) {
                old  = head;
                head = head->next;
                delete old;
            }
        }
        void pushbackNode(Node* node)
        {
            if (tail) {
                tail->next = node;
                tail = tail->next;
            }
            else {
                head = tail = node;
            }
        }
};


Creating a grid to the specified size using lists My code creates a "grid" of empty places, where 0 is used as the place holder, in a size specified by the user.

I was wondering if there were more efficient ways of doing this,or perhaps ways that make more aesthetically pleasing grids with less code.

My code is in working order and does as it says on the label, I also implemented a system that does not allow for incorrect inputs.

def gridSize_def():
    global gridSize
    while True:
        try:
            gridSize = int(input("input grid length and width\n"))
            if gridSize<2:
                print ("invalid int")
            else:
                break
        except ValueError:
            print ("invalid input")
    grids = [[] for _ in range (gridSize)]
    for i in range(0,gridSize):
        grids[i-1]=[0 for _ in range(gridSize)]
    return grids

def grid_def():
    for i in range (0,gridSize):
        print (grids[i-1])

grids = gridSize_def()
grid_def()

All feedback welcome!

Grid creation

You know about list-comprehensions, but you use a default value before filling your rows with zeroes. You could do it in one go:

grids = [[0 for _ in range(gridSize)] for _ in range(gridSize)]

which can be simplified to

grids = [[0] * gridSize for _ in range(gridSize)]

Grid printing

The for loop in Python is meant to be used to iterate directly over elements rather than indices:

for row in grids:
    print(row)

Function parameters

It is bad practice to rely on global to share data accros functions. You should define those function to accept parameters instead.

This let you also extract input handling from your grid creation function which is also better as let you have a single responsibility per function:

def ask_size():
    while True:
        try:
            grid_size = int(input("input grid length and width\n"))
            if grid_size < 2:
                print ("invalid int")
            else:
                return grid_size
        except ValueError:
            print ("invalid input")

def make_grid(size):
    return [[0] * size for _ in range(size)]

def print_grid(grid):
    for row in grid:
        print(row)

grids = make_grid(ask_size())
print_grid(grids)

Exception handling

Python have else clauses that are executed when the try part succeeded. The aim is to reduce the amount of code protected by the try to avoid catching unexpected exceptions. In your case, you could change that part to:

def ask_size():
    while True:
        try:
            grid_size = int(input("input grid length and width\n"))
        except ValueError:
            print ("invalid input")
        else:
            if grid_size > 1:
                return grid_size
            print ("invalid int")


Accessing tweets from Twitter API I need help making this code more efficient but still functions the same, such as by making it access the Twitter API fewer times. When I run this code for like 1 min, I get an error from Twitter saying that it exceeds limits.

List<Status> tweets = result.getTweets();

       for (Status tweet : tweets) {

            if(!(tweet.getUser().getScreenName().toString().equals("gh") || tweet.getUser().getScreenName().toString().equals("gh"))){

                boolean found = false;

                List<Status> Stats=  tw.getHomeTimeline();
                for (Status StatList : Stats) {

                    if(StatList.getTweetText().equals(tweet.getTweetText())){
                        found = true;
                        break;
                        try {
                            System.out.println(tweet.getUser().getScreenName() );

                            Thread.sleep(2000);
                        } catch(InterruptedException ex) {
                            Thread.currentThread().interrupt();
                        }
                    }else{
                        // nothing here
                    }
                }
                if (found) // you can move this and the initialisation(boolean found = false) out side the upper if statement
                    System.out.println("false");
                else
                    System.out.println("turn");

            }else{
                System.out.println("blocked");
            }
        }

Just a few hints that might better the code:

If you tend to access a structure for a given element multiple times with an operation on it, extract it in a local variable:

tweet.getUser().getScreenName() // If its String, toString not required.

Have try..catch for statements that can cause the exception i.e put this out

System.out.println(tweet.getUser().getScreenName() );

As your comment states, put the boolean out with the two states and have minimal print statements. If its for debugging, add comments or debug flag.
Having try..catch after break? Please check.
Check if tweets is null after

List<Status> tweets = result.getTweets();

Check if tweet is null inside the for loop.
Apply (5) and (6) for Stats as well.


Validator refactored to be OO Based on my previous post I completely re-factored my code to make it object oriented.

Anything I could improve when it comes to:

Object oriented code;
Efficiency;
Readability.

Any other suggestions are also welcome!

Validator class:

class Validator
{
    private $rules  = [];
    private $errors = [];

    public function getErrors($rulename = null)
    {
        if ($rulename) {
            return isset($this->errors[$rulename]) ? $this->errors[$rulename] : null;
        }

        return $this->errors;
    }

    # rules will be checked against in same order as specified
    public function setRule($name, Array $rules)
    {
        if (!array_key_exists($name, $this->rules)) {
            foreach ($rules as $rule) {
                if (!$rule instanceof IRule) {
                    throw new Exception('Array must only contain instances of IRule.');
                }
            }

            $this->rules[$name] = $rules;
        } else {
            throw new Exception(sprintf('Rule %s already exists.', $name));
        }
    }

    public function validate($rulename, $input)
    {
        foreach ($this->rules[$rulename] as $rule) {
            try {
                $rule->check($input);
            } catch (Exception $e) {
                $this->errors[$rulename][] = $e->getMessage();
            }
        }

        if (isset($this->errors[$rulename])) {
            return false;
        }

        return true;
    }
}

One of the rule classes:

class Between implements IRule
{
    private $errMsg = 'Must be between %s and %s.';

    private $inclusive;
    private $max;
    private $min;

    public function __construct($min, $max, $inclusive = true, $errMsg = null)
    {
        $this->min       = $min;
        $this->max       = $max;
        $this->inclusive = $inclusive;

        if ($errMsg !== null) {
            $this->errMsg = $errMsg;
        }
    }

    public function check($input)
    {
        if ($this->inclusive && $input >= $this->min && $input <= $this->max) {
            return true;
        } elseif (!$this->inclusive && $input > $this->min && $input < $this->max) {
            return true;
        } else {
             throw new Exception(sprintf($this->errMsg, $this->min, $this->max));
        }
    }
}

Example:

$validator = new Validator();

$validator->setRule('name',  [new NotEmpty(), new MaxChars(20), new Alpha(true)]);
$validator->setRule('email', [new NotEmpty(), new Email()]);

$validator->setRule('age',
    array(
        # last constructor argument overrides default error message
        new NotEmpty('You did not enter your age.'),
        new Numeric('Your age can only be numerical.'),
        new Between(13, 17, true, 'Only teenagers are allowed.')
    )
);

$validator->validate('name',  'John');
$validator->validate('email', 'Doe');
$validator->validate('age',   'Hello');

# report
if (!$validator->getErrors()) {
    echo 'Input(s) valid: Passed complete validation.';
} else {
    echo 'Input(s) invalid: Failed complete validation.';
}

This is a really solid start. I only have a couple of suggestions.

Instead of defining the rules outside of the Validator class, it might be a good idea to sub class Validator and set the rules inside the constructor.
Create public methods specifically for each kind of validator.

Some pseudo code to get the main idea:

validator.php

class Validator
{
    private $rule_collection = array();

    public function __construct()
    {
    }

    public function ruleFor($property)
    {
        if (!isset($this->rule_collection[$property]))
            $this->rule_collection[$property] = new RuleBuilder($property);

        return $this->rule_collection[$property];
    }

    public function validate($model)
    {
        $valid = true;

        foreach ($this->rule_collection as $property => $rule_builder)
        {
            foreach ($rule_builder->rules as $rule)
            {
                if (!$rule->validate($model->$property))
                    $valid = false;
            }
        }

        return $valid;
    }
}

rule_builder.php

interface IRule
{
    public validate($value);
    public get_message();
}

class RuleBuilder
{
    private $property;
    public $rules = array();

    public function __construct($property)
    {
        $this->property = $property;
    }

    public function isRequired()
    {
        return $this->addRule(new RequiredRule());
    }

    public function length($min, $max = -1)
    {
        return $this->addRule(new LengthRule($min, $max));
    }

    public function addRule(IRule $rule)
    {
        $this->rules[] = $rule;
        return $this;
    }
}

And your specific validator:

class UserValidator extends Validator
{
    public function __construct()
    {
        $this->ruleFor('name')
            ->isRequired()
            ->length(20);
    }
}

To use it:

$user = new User();
$user->name = "Foo";
$validator = new UserValidator();

if ($validator->validate($user))
    // save
else
    // redraw form and show errors

This way all of your rule classes are hidden. All of the validation rules are portable because they are created inside a model specific class, and it's easy to use an IDE's auto complete feature to discover which rules are available since the RuleBuilder class has strongly typed methods encapsulating each rule.

On top of that, the IRule interface just accepts a value and not a model, making each rule unit testable to ensure your validation library is functioning properly.

And for those who do some .NET/C# development, this pattern probably looks familiar if you've used the FluentValidation NuGet package for Visual Studio. Say what you want about .NET development, but there are some gems out there. I do like the pattern that FluentValidation uses.


Sum of number's quotients when divided by exact powers of 2 Problem
I have written code to solve this challenge:

Given a formula:

GCD(x,y) means the GCD (Greatest Common Divisor) of x and y.
For example: if N=2, then:

f(2) = 1/GCD(1, 4) + 2/GCD(2, 4) + 3/GCD(3, 4) + 4/GCD(4,4)
f(2) = 1/1 + 2/2 + 3/1 + 4/4
f(2) = 1 + 1 + 3 + 1
f(2) = 6

Given N, find f(N).
Output:
Value of f(N) modulo 1.000.000.007 (109 + 7)
Edit: Output should be in modulo 10^9 + 7

I found a simplified formula using Wolframalpha, by finding the first 10 elements: (4^(n + 1) + 8) / 12
Code
#include <bits/stdc++.h>
#define MOD 1000000007
using namespace std;
typedef short i16;
typedef unsigned short u16;
typedef int i32;
typedef unsigned int u32;
typedef long int i64;
typedef unsigned long int u64;
typedef float f32;
typedef double f64;

u64 pawa(u64 a, u64 b){
    u64 tot = 1;
    for (u64 i = 0; i < b; i++) {
        tot *= a;
        tot = tot % MOD;
    }
    return tot;
}

int main(){
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    u64 n;
    cin>>n;
    cout<<(pawa(4, n + 1) + 8) / 12<<'\n';
    return 0;
}

This code scales poorly - time limit exceeded. The upper corner constraint is 10^9. How do I improve my algorithm?

It's great that you found a closed form to generate the results.  Shame that it was by search rather than by reasoning.  (i/GCD(i, 2^n) is simply i shifted right until it's odd, so we get the sequence (in binary) 1**,** 10, 11, 100, 101, 110, 111, 1100, ...  You can see that every other value is divided by 2, every fourth value divided by 4 and so on; that allows you to arrive at the closed form fairly quickly.

I'll note that the numerator and denominator have a common factor of 4, so could be reduced to \$\frac{4^n+2}3\$.

Now to the code:

<bits/stdc++.h> is non-portable, and extremely wasteful.  Include only the headers that are needed; in this case just <iostream>.

Instead of using a preprocessor #define for a constant integer, always prefer to use a C++ constant.  Macros don't respect context.

Avoid using namespace std;.  It doesn't even save you typing, once the unnecessary stdio operations are removed.

The typedefs are misleading: the name u64 suggests that unsigned long happens to be 64 bits on your particular target, but it also suggests that you've written brittle code that depends on that assumption (and all the other typedefs appear to be unused).  For this code, I think we really should be using std::uint_fast64_t from <cstdint>.

Moving on to main(): as hinted earlier, std::ios_base::sync_with_stdio() and std::cin::tie() are pointless, given that we're not using the C standard streams, so should be omitted.  Also, there's no checking that streaming from std::cin was successful; that's easily fixed.

Now we need to look at pawa().  Given that we only ever call it with a equal to 4, then we can make a special-purpose function rather than a general modpow() - let's call it exp4mod() (meaning exponentiation base 4, modulo 1000000007).

For values of b less than 15, the result is less than 1000000007, so that is easily computed as 1u << (2*b).  Values of 15 or more can be reduced using the identity \$4^{14a+b} = (4^{14})^a4^b\$, and then we can use a standard binary exponentiation for the first part and a shift for the second.

All that said, it's unclear why the exponentiation must be done modulo 1000000007 - the description says that the result be reduced modulo such a number.  So perhaps there's a logic error there, and we really need to be reducing modulo 3000000021 (having reduced the fraction so we divide by 3)?

Here's the version I ended up with, having made the recommended changes (and modifying main() not to require input):

#include <cstdint>
#include <iostream>

using u64 = std::uint_fast64_t;

static u64 modpow(u64 x, u64 y, u64 mod)
{
    u64 result = 1;
    while (y) {
        if (y%2) {
            result = result * x % mod;
        }
        x = x * x % mod;
        y /= 2;
    }

    return result;
}

static u64 exp4mod(u64 x)
{
    static auto const mod = 3000000021;
    static auto const chunk_size = 16;

    static const u64 one = 1;
    static auto const residue = (one << 2*chunk_size) % mod; // 4^15%mod = 73741817

    const auto a = x / chunk_size;
    const auto b = x % chunk_size;
    return (modpow(residue, a, mod) << (2 * b)) % mod;
}

int main()
{
    for (u64 n = 0;  n < 35;  ++n)
        std::cout << n << ": " << (exp4mod(n) + 2) / 3 << '\n';
}


Dynamic matrix in web browser Today, my project was to make a matrix in my web browser. It is really slow, so help with optimizing it would be nice. It runs slow on my PC, yet alone my iPod 4 which this is going to be for.

    <title>Matrix</title>

     hi hi

You can try it out here. ANY speed improvements would greatly help!

Here's what I did

var theMatrix = (function (containerId, lines, columns) {
    // We wrap everything in a closure, and expose only a single
    // global. Avoids global pollution, and keeps everything in
    // one place.

    var characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';

    // We break functionality into functions, naming verbosely
    function randomInt(min, max) {
        return Math.floor(Math.random() * (max - min + 1)) + min;
    }

    function randomGreen() {
        return "#00" + randomInt(0, 255).toString(16) + "00";
    }

    function randomLetter() {
        return characters.charAt(Math.floor(Math.random() * characters.length));
    }

    function generateLine(columns) {
        var line = document.createElement('div');
        while (columns--) {
            var letter = document.createElement('span');
            letter.style.color = randomGreen();
            letter.innerHTML += randomLetter();
            line.appendChild(letter);
        }
        return line;
    }

    function writeLines(container, lines, columns) {
        for (var i = 0; i < lines; i++) {
            container.appendChild(generateLine(columns));
        }
    }

    function startMatrix(container,columns) {
        setInterval(function loop() {
            // Instead of rebuilding the DOM all at once, we change
            // by line, by removing the top line, and appending
            // at the bottom.
            var firstLine = container.firstChild;
            container.removeChild(firstLine);
            container.appendChild(generateLine(columns));

            // Also, used doesn't care, you can replace the line
            // before this with the following line to cycle the
            // lines instead of regenerating the last line
            //container.appendChild(firstLine);
        }, 50);
    }

    // The exposed function. The only function that "keeps state"
    // (knows instance settings). All other function are reusable
    // and are not bound to a single call
    return function (containerId, lines, columns) {
        var container = document.getElementById(containerId);

        // we write the initial lines
        var textNodes = writeLines(container, lines, columns);

        // start the matrix
        startMatrix(container,columns);
    };
}());

theMatrix('mat', 50, 80);


Creating a list of pairs after splitting the string containing all of the sets I am working on asp.net/MVC and I need to read a value from my web.config file. Splitting the list into multiple keys is not an option.

<add key="DepartAssign" value="Depart1:PersonAssignTo1; Depart2:PersonAssignTo2"/>

I handled the problem by doing the following (which I am sure that there is a better approach)

public List<string> departmentList = new List<string>();
public List<string> assignedtoList = new List<string>();
public string[] pair = ConfigurationManager.AppSettings["DepartAssign"].Split(';');
foreach (string s in pair)
{
     departmentList.Add(s.Split(':').First()); //add the department to the list
     assignedtoList.Add(s.Split(':').Last()); //add the assignedto to the list
 }

and of course, I would be able to match the department with the assigned person by using the same index. Not really the cleanest approach but it works for now.

any way to make this code better?

You could create your own custom section in your config file.

First define what each record looks like:

public class Department : ConfigurationSection
{
    [ConfigurationProperty("name", IsRequired = true)]
    public string Name
    {
        get { return (string)this["name"]; }
        set { this["name"] = value; }
    }

    [ConfigurationProperty("assignee", IsRequired = true)]
    public string Assignee
    {
        get { return (string)this["assignee"]; }
        set { this["assignee"] = value; }
    }
}

Define the a collection so the framework can work with your section

public class DepartmentCollection : ConfigurationElementCollection
{
    public override ConfigurationElementCollectionType CollectionType
    {
        get { return ConfigurationElementCollectionType.BasicMap; }
    }

    protected override string ElementName
    {
        get { return "Department"; }
    }

    protected override ConfigurationPropertyCollection Properties
    {
        get { return new ConfigurationPropertyCollection(); }
    }

    public Department this[int index]
    {
        get { return (Department) BaseGet(index); }
        set
        {
            if (BaseGet(index) != null)
            {
                BaseRemoveAt(index);
            }
            base.BaseAdd(index, value);
        }
    }

    public Department this[string name]
    {
        get { return (Department) BaseGet(name); }
    }

    public void Add(Department item)
    {
        base.BaseAdd(item);
    }

    public void Remove(Department item)
    {
        BaseRemove(item);
    }

    public void RemoveAt(int index)
    {
        BaseRemoveAt(index);
    }

    protected override ConfigurationElement CreateNewElement()
    {
        return new Department();
    }

    protected override object GetElementKey(ConfigurationElement element)
    {
        return (element as Department).Name;
    }
}

Define what the list looks like

public class DepartmentList : ConfigurationSection
{
    private static readonly ConfigurationPropertyCollection DepartmentListProperties;
    private static readonly ConfigurationProperty DepartmentProperty;

    static DepartmentList()
    {
        DepartmentProperty = new ConfigurationProperty(
            "",
            typeof (DepartmentCollection),
            null,
            ConfigurationPropertyOptions.IsRequired | ConfigurationPropertyOptions.IsDefaultCollection
            );

        DepartmentListProperties = new ConfigurationPropertyCollection
                                       {
                                           DepartmentProperty
                                       };
    }

    public DepartmentCollection Departments
    {
        get { return (DepartmentCollection) base[DepartmentProperty]; }
        set { base[DepartmentProperty] = value; }
    }

    protected override ConfigurationPropertyCollection Properties
    {
        get { return DepartmentListProperties; }
    }
}

Now, in your .config file specify the new type:

<configSections>
    <section name="DepartmentAssigneeMapping" type="DatabaseCleanup.DepartmentList, DatabaseCleanup, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"/>
</configSections>

and add your section

<DepartmentAssigneeMapping>
    <Department name="Depart1" assignee="PersonAssignTo1" />
    <Department name="Depart2" assignee="PersonAssignTo2" />
</DepartmentAssigneeMapping>

Now, in your class:

public List<string> departmentList;
public List<string> assignedtoList;

var mappings = (DepartmentList) ConfigurationManager.GetSection("DepartmentAssigneeMapping");

departmentList = (
    from Department department in mappings.Departments
    select department.Name).ToList();
assignedtoList = (
    from Department department in mappings.Departments
    select department.Assignee).ToList();

Your class is much nicer, and the config is way cleaner.

This is fully tested, and the results you are looking for are replicated.


Conditionally display an Action depending on whether or not a player is connected Is there a nicer way to write the maybeWhen portion of displayConnectionAction ? I want to conditionally display an Action depending on whether or not the player is connected.

This bit of code is lifted from a wider system where there are multiple calls like displayConnectionAction (conditional actions built up by a Reader GameState), so I am not after any feedback around things like the Reader being overkill etc, more the implementation of the function displayConnectionAction, the actual code has 12+ functions like this, all generating Maybe Actions based off various bits of logic.

data Action = Action
  { actionLabel :: Text
  , actionUrl :: Text
  , actionPlayer :: Int
  }

data GameState = GameState
  { currentPlayer :: Player
  }

data Player = Player
  { playerId :: Int
  , playerName :: Text
  , playerConnected :: Bool
  }

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = maybeWhen not <$> isConnected <*> action
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

maybeWhen :: (a -> Bool) -> a -> Maybe b -> Maybe b
maybeWhen f a mb =
  if f a then mb else Nothing

It always looks a little odd when one has to 'invent' something that seems like it ought to be a common, built-in expression, so I can understand why you ask.

Please interpret any of the following as nothing but my humble attempt to share what I know about Haskell programming. I don't consider myself a Haskell expert, and I don't know if other people would consider any of the following idiomatic...

Simplify maybeWhen

Is it necessary to pass both a predicate and a value (a) when the value itself turns out to be a Boolean value?

Would it make sense to simplify maybeWhen to something like the following?

maybeWhen' :: Bool -> Maybe b -> Maybe b
maybeWhen' b mb =
  if b then mb else Nothing

This would enable you to rewrite displayConnectionAction like this:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = maybeWhen' <$> (not <$> isConnected) <*> action
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

All of the function body (from where and down) is the same; only the composition is different.

Notice that it first maps isConnected with not, and the applies the projected value and action to the simplified maybeWhen' function.

maybeWhen' is just a composition

Instead of writing a function like maybeWhen', you can compose it from bool from Data.Bool:

Prelude Data.Bool> :t flip (bool Nothing)
flip (bool Nothing) :: Bool -> Maybe a -> Maybe a

Plugging that into displayConnectionAction means that you can now write it like this:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = flip (bool Nothing) <$> isConnected <*> action
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

Again, all code below where is the same. Notice, however, that you no longer need any helper function.

Or, instead of using the flip function, you could just flip the arguments:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = bool Nothing <$> action <*> isConnected
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

Notice that in this version, action and isConnected has changed places.

All this, however, relies on the simplification that in this particular example, the a in maybeWhen turns out to be Bool. Perhaps that's not always the case...

Use do-notation

Personally, I think this would be more readable if you rewrite the function using do notation. There are definitely Haskell programmers who consider do notation a crutch that has 'nothing to do with functional programming', but I don't share that sentiment.

Since Reader is a Monad, you could refactor the function to this implementation:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = do
  currentPlayerId <- asks (playerId . currentPlayer)
  let action = Just . Action "Connect" "/connect" $ currentPlayerId
  isConnected <- asks (playerConnected . currentPlayer)
  if not isConnected
    then return action
    else return Nothing

Instead of having various Reader GameState values that you need to figure out how to combine, a variable like currentPlayerId is now simply an Int, and isConnected is simply a Bool.

Ultimately, though, if you often find yourself short-circuiting Bool values to Nothing, you may want to consider using guard, when, and unless with the Maybe monad. In this case, though, this would require you to use a monad transformer so that you can either stack your Reader inside MaybeT, or stack Maybe inside ReaderT.


Website registration Can you please tell me how to improve this code?

<?php
 include 'configdb.php';

 if(($_SERVER['REQUEST_METHOD'] === 'POST') && !empty($_POST['email']) && !empty($_POST['password']))
 {
    $getUserData = getUserByEmail();
    if (mysqli_stmt_num_rows($getUserData) > 0)
    {
        die(
          '{
            "status":"1",
            "message":"Email Already Exists"
           }'
        );
    }
    else
    {
        insertUser();
        die('{
              "status":"0",
              "message":"Registration Successful"
          }');
    }
 }

  function insertUser()
  {
    $email = $_POST['email'];
    $password = password_hash($_POST['password'], PASSWORD_DEFAULT);

    $insertData = mysqli_prepare($GLOBALS['connection'], "INSERT INTO users (emailID, password, com_code) VALUES(?,?,?)");

    if(false == $insertData)
    {
        handlePrepareStmtErrors("-1");
    }
    $bindParam = mysqli_stmt_bind_param($insertData, "sss", $email, $password, $com_code);
    if(false == $bindParam)
    {
        handlePrepareStmtErrors("-2");
    }
    $executeStmt = mysqli_stmt_execute($insertData);
    if(false == $executeStmt)
    {
        handlePrepareStmtErrors("-3");
    }
   }

   function getUserByEmail()
   {
    $getUserData = mysqli_prepare($GLOBALS['connection'],"SELECT emailID FROM users WHERE emailID = ?");
    if(false == $getUserData)
    {
        handlePrepareStmtErrors("-1");
    }
    $bindParam = mysqli_stmt_bind_param($getUserData,"s",$_POST['email']);
    if(false == $bindParam)
    {
        handlePrepareStmtErrors("-2");
    }
    $executeStmt = mysqli_stmt_execute($getUserData);
    if(false == $executeStmt)
    {
        handlePrepareStmtErrors("-3");
    }
    mysqli_stmt_store_result($getUserData);/*It Is Required To Get Number Of Rows By stmt_num_rows Method */
    return $getUserData;
   }

   function handlePrepareStmtErrors($code)
   {
    switch((String)$code)
    {
        case "-1":
            die('{
                "status":"-1",
                "message":"Statement prepare error"
            }');

        case "-2":
            die('{
                "status":"-1",
                "message":"Parameter binding error"
            }');
        case "-3":
            die('{
                "status":"-1",
                "message":"Statement execution error"
            }');
      }
    }
   ?>

Things about which I am not sure:

I read that MySQLi prepare statements will prevent SQL injections. Is there any possibility to hack my db by my design?
I am planning to create a utils.php and will add handlePrepareStmtErrors function there. In that function, I am exiting the scripts if any errors are there. So only I am not using break statement there because it will not get executed. Is it right way?
I feel like my functions are not properly written, but it does what I want. I tried and registration is successful.

I read that MySQLi prepare statements will prevent SQL injections. Is there any possibility to hack my db by my design?

No, there shouldn't be a way. Prepared statements are the recommended way to prevent SQL injection, and you are using them correctly and consistently.

  I am planning to create a utils.php and will add handlePrepareStmtErrors function there. In that function, I am exiting the scripts if any errors are there. So only I am not using break statement there because it will not get executed. Is it right way?

Yes, you don't need break if you are exiting or returning. Having a default is still a good idea though.

But you shouldn't exit. Helper functions and classes in general should not exit/die, they should throw exceptions or return false. If you just die, the calling function has no change to recover, thus making the function hard to reuse.

  I feel like my functions are not properly written, but it does what I want. I tried and registration is successful.

I think that your feeling is correct.

I could for example imagine multiple usages for the function getUserByEmail, but the way you are writing it, it can't really be reused, as it needs the POST value email to be set.

Ideally, the signature would look like this: getUserByEmail($email). I also just noticed that it doesn't actually get the user, so doesUserExist($email) which then returns true or false would be more fitting.


Form a string consisting of the common letters that appear in two input strings The idea is to write a function that takes two strings and returns a new string that repeats in the previous two: examples:

'ABBA' & 'AOHB' => 'AB'
'cohs' & 'ohba' => 'oh'

A brute force solution would be nested for loops like so:

const x = 'ABCD'
const y = 'AHOB'

function subStr(str1, str2) {
  let final = ''

  for (let i = 0; i < str1.length; i++) {
    for (let j = 0; j < str2.length; j++) {
      if (str1[i] === str2[j]) {
        final += str1[i]
      }
    }
  }

  return final
}

console.log(subStr(x, y)) // => AB

I know the root of all evil is premature optimization, but I would first ask if this is expected to be either a hotspot or usef with large strings.

Because it can be made much more readable using the array methods, but doing so obviously comes with a performance cost. OTOH, your method has its own performance issues.

If this isn’t expected to be particularly performance sensitive, I think that a straightforward method that turned the two strings into arrays then used the filter or map method to generate an array that is then turned into a string would be much more readable, and I prefer readable over performant as long as the performance isn’t a problem.

One additional point, what should your function return for “a”, “aa”? currently it returns “aa”?


Simple chatbot written in Python I wrote a simple chatbot in Python a while ago, and I'd like to know how it could be improved. Here's the code:

import random
import pickle

class Bot:
    current = ""
    botText = "BOT> "

    def __init__(self, data, saveFile):
        self.data = data
        self.saveFile = saveFile

    def say(self, text):
        print(self.botText + text)
        self.current = text

    def addResponse(self, userInput, response):
        if userInput in self.data:
            self.data[userInput].extend(response)

        else:
            self.data[userInput] = []
            self.data[userInput].extend(response)

    def evaluate(self, text):
        if text in self.data:
            self.say(random.choice(self.data[text]))

        elif text == "/SAVE":
            f = open(self.saveFile, 'wb')
            pickle.dump(self.data, f)
            f.close()

        elif text == "/LOAD":
            f = open(self.saveFile, 'rb')
            self.data = pickle.load(f)
            f.close()

        elif text == "/DATA":
            print(self.data)

        else:
            if not self.current in self.data:
                self.data[self.current] = []

            self.data[self.current].append(text)
            self.say(text)

Here's how the bot works. I'm sorry if you don't understand, I'm not the best at explaining things.

User enters input.
If the input is in the database, choose a random response associated with the input and output it.
If it isn't, add the input to the database and echo the input.
User enters input again.
The input is associated with the bot's output.

Inputs and outputs can be added manually using the addResponse() function.

There are also a few commands, which are quite self-explanatory, but I'll list them here anyways.

/SAVE pickles the file and saves it in the saveFile.
/LOAD unpickles the saveFile and loads it.
/DATA displays the database.

Clean and straightforward, very nice.

Minor suggestions:

If you do

from collections import defaultdict

...

self.data = defaultdict(list)

then you can make addResponse be just:

self.data[userInput].extend(response)

and likewise you can leave out the if block under the else clause in evaluate
The code:

f = open(self.saveFile, 'rb')
self.data = pickle.load(f)
f.close()

is slightly-better written as:

with open(self.saveFile, 'rb') as f:
    self.data = pickle.load(f)

so that if pickle.load() throws an exception, the file will still get closed.  Similar for the /SAVE code.