Validating an entity using a dynamic list of predicates I have a generic extension method for validating my entities. The main idea is to be able to specify at runtime (context related) the criteria for validating a specific entity (with the end goal of saving it in the data base).

public static Boolean IsValidEntity<T>(this T entity, List<Predicate<T>> predicates)
{
    Boolean res = true;
    predicates.ForEach(p => res &= (Boolean)p.DynamicInvoke(entity));
    return res;
}

Inside my repository I can then use (simplified as this is not the scope but is somewhat  necessary for the question context):

public void SaveIntProperty(int myProperty, List<Predicate<int>> predicates)
{
    if (myProperty.IsValidEntity<int>(predicates))
    {
        //save to data base or other actions
    }
}

A simple example on a list of ints would be to search for elements which (in a particular context) need to be both even and less than 8.

List<int> numbers = new List<int>() { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

//dynamically create the validation criteria
List<Predicate<int>> predicates = new List<Predicate<int>>();// List<Delegate>();
Predicate<int> lessThan = x => x < 8;
predicates.Add(lessThan);
Predicate<int> even = x => x % 2 == 0;
predicates.Add(even);

numbers.ForEach(n => SaveIntProperty(n, predicates));

The IsValidEntity extension method works and looks simple enough. However, I'm having doubts about the performance and the extensibility. Is the use of the ForEach ok? Should I pass the predicates differently? Should I use something else instead of predicates?

You can simplify your extension method a bit as so:

public static Boolean IsValidEntity<T>(this T entity, IEnumerable<Predicate<T>> predicates)
{
    return predicates.All(p => (bool)p.DynamicInvoke(entity));
}

This is a bit better for readability, and shouldn't have any cost in efficiency as LINQ short-circuits with All. I also switched your List for IEnumerable, just to save an unnecessary ToList call if you end up with some other kind of collection.

You should also check if you really need that DynamicInvoke. If not, you can further simplify to

public static Boolean IsValidEntity<T>(this T entity, IEnumerable<Predicate<T>> predicates)
{
    return predicates.All(p => p(entity));
}

Depending on your wider design, this might be enough. However, since you asked about extensibility, you can see that having to build predicates isn't the nicest business. For example, you had your 'less than 8' predicate. What if elsewhere in the code you wanted 'less than 7'? You'd have to repeat the (albeit very simple in this example) logic, just with a different number.

If you tried to refactor this to adhere to the Don't Repeat Yourself principle, you might then end up with something like a PredicateFactory class which exists purely to build common predicates based on parameters. But this isn't fantastic design either. It would likely end up as a monolith of public methods without much cohesion, and violate the open/closed and single responsibility principle.

A better option might be to create an interface like:

interface IEntityValidator<T>
{
    bool IsValid(T Entity);
}

Instead of predicates, you can now create and use classes which implement this interface. This gives better adherence to the design principles I mentioned before, and allows you take full advantage of polymorphism (for more complicated validation you might find that inheritance is useful, for example). At this point your original method would be:

public static Boolean IsValidEntity<T>(this T entity, IEnumerable<IEntityValidator<T>> validators)
{
    return validators.All(v => v.IsValid(entity));
}

To my mind, that line is simple and readable enough that it may no longer warrant its own method, especially its own extension method which will apply to every single type. Validation doesn't seem like something that would be done in too many places, so even if you didn't like that line, you might just move it to a private method somewhere. That's more a matter of taste, though.


Responsive CSS, abbreviated content I wonder if there is a "correct" way to display different content, based on the the state of responsive webdesign. Different in this case is meant as an abbreviated text to display instead of the full one.

I have created this example table with a couple columns, containing either the full name of a state, or, when resized to a smaller screensize, it's 2-letter code.

HTML:

<!doctype html>
<meta name="viewport" content="width=device-width"/>

    CaliforniaCA
    FloridaFL
    TexasTX
    GeorgiaGA

    Sacramento
    Tallahassee
    Austin
    Atlanta

CSS:

th {
    border: 1px solid #000;
}

td {
    border: 1px solid #000;
}

@media screen and (max-width:760px){
.full {
    display: inline;
    }
.abbr {
    display: none;
}
}

@media screen and (max-width:320px){
.full {
    display: none;
    }
.abbr {
    display: inline;
}
}

jsFiddle

It works as I want, but this solution kind of seems more like a hack to me to get it working.

I've googled, but haven't found anything regarding the topic, which might just be the result of not knowing what exactly to ask. Anyhow, I somehow got the feeling, that there's a more sophisticated way of doing such a thing.

To expand on Hubert Grzeskowiak's answer the custom data attribute is a good option for you as you can continue specifying content in the markup as it should.

But there is a way with CSS to display it with the content property. Used on a pseudo element, you can choose to show it or not depending on viewport width with media queries.

This will allow you to :

minimize markup
change the content according to the viewport width
no use of JS
keep the content in the markup

DEMO

th {
    border: 1px solid #000;
    position:relative;
}
td {
    border: 1px solid #000;
}
.full:after{
    content: attr(data-state);
    display:none;
}
@media screen and (max-width:320px) {
    .full:after{
        position:absolute;
        top:0;left:0;
        width:100%; height:100%;
        background:#fff;
        display:block;
}

        California
        Florida
        Texas
        Georgia

        Sacramento
        Tallahassee
        Austin
        Atlanta


Resository Pattern and Unit of Work with ADO.net I am trying to implement the Repository Pattern with a Unit of work but I don't know if I am on the right direction. This is my Model:

public class Resource
{
    public string RessourceUID { get; set; }
    public string RessourceName { get; set; }
    public List<Team> Teams { get; set; }
    public string Loginname { get; set; }
}

This is my Repository:

public class ResourceRepository
{
    public static Resource GetResourceByLoginname(string loginname)
    {
        return RunWithElevatedPrivileges(() => GetResourceByLoginnameInternal(loginname));
    }

    private static Resource GetResourceByLoginnameInternal(string loginname)
    {
        return ExecuteSql(cmd =>
        {
            Resource resource;

            cmd.CommandText = @"SELECT convert(nvarchar(36), ResourceUID) AS ResourceUID, UserClaimsAccount, ResourceName FROM MSP_EpmResource_UserView WHERE UserClaimsAccount = @userclaimsaccount";
            cmd.Parameters.AddWithValue("@userclaimsaccount", SqlDbType.NVarChar).Value = loginname;

            using (var dr = cmd.ExecuteReader())
            {
                if (!dr.HasRows)
                    return null;

                resource = new Resource();

                while (dr.Read())
                {
                    MapResourceData(resource, dr);
                    resource.Teams = RunWithElevatedPrivileges(() => GetResourceTeams(resource));
                }
            }

            return resource;
        });

    }

     private static void MapResourceData(Resource resource, SqlDataReader dr)
    {
        resource.RessourceUID = SqlReaderHelper.GetValue<string>(dr, "ResourceUID");
        resource.RessourceName = SqlReaderHelper.GetValue<string>(dr, "ResourceName");
        resource.Loginname = SqlReaderHelper.GetValue<string>(dr, "UserClaimsAccount");
    }

    private static T RunWithElevatedPrivileges<T>(Func<T> action)
    {
        var result = default(T);

        Microsoft.SharePoint.SPSecurity.RunWithElevatedPrivileges(() => result = action());

        return result;
    }

    private static void ExecuteSql(Action<SqlCommand> action)
    {
        string connectionString = Connectivity.GetConnectionString();

        using (var conn = new SqlConnection(connectionString))
        {
            conn.Open();
            var cmd = conn.CreateCommand();

            action(cmd);
            cmd.ExecuteNonQuery();
        }
    }

    private static T ExecuteSql<T>(Func<SqlCommand, T> action)
    {
        T result;

        string connectionString = Connectivity.GetConnectionString();

        using (var conn = new SqlConnection(connectionString))
        {
            conn.Open();
            var cmd = conn.CreateCommand();

            result = action(cmd);
            cmd.ExecuteNonQuery();
        }

        return result;
    }
}

Is this a good design?
Is this the Repository Pattern?
What could I optimize?
I absolutely don't know how to implement the Unit of work. How could I do this?
Should I move the ExecuteSql methods out of the Repository?
Should I also move the MapXXXData methods out of this Repository?

It's a good start. Here are the changes I suggest:

I think you should be able to execute the queries without sharepoint. To make it possible we'll remove the sharepoint stuff from the repository and put it in another one. We then combine them with a decorator pattern like this:

The first step is to extract an interface (or an abstract class):

public interface IResourceRepository
{
    Resource GetByLoginname(string loginname);
}

Then we implement it in the first repository that will get resources from the database:

public class ResourceRepository : IResourceRepository
{
    public Resource GetByLoginname(string loginname)
    {
        return ExecuteSql(cmd =>
        {
            cmd.CommandText = @"SELECT convert(nvarchar(36), ResourceUID) AS ResourceUID, UserClaimsAccount, ResourceName FROM MSP_EpmResource_UserView WHERE UserClaimsAccount = @userclaimsaccount";
            cmd.Parameters.AddWithValue("@userclaimsaccount", SqlDbType.NVarChar).Value = loginname;

            using (var reader = cmd.ExecuteReader())
            {
                while (reader.Read())
                {
                    var resource = Resource.From(reader);
                    if (resource != null)
                    {
                        resource.Teams = GetResourceTeams(resource);
                    }
                    return resource;
                }
                return null;
            }
        });
    }

    private static void ExecuteSql(Action<SqlCommand> action)
    {
        string connectionString = Connectivity.GetConnectionString();

        using (var conn = new SqlConnection(connectionString))
        {
            conn.Open();
            var cmd = conn.CreateCommand();

            action(cmd);
            cmd.ExecuteNonQuery();
        }
    }

    private static T ExecuteSql<T>(Func<SqlCommand, T> action)
    {
        T result;

        string connectionString = Connectivity.GetConnectionString();

        using (var conn = new SqlConnection(connectionString))
        {
            conn.Open();
            var cmd = conn.CreateCommand();

            result = action(cmd);
            cmd.ExecuteNonQuery();
        }

        return result;
    }
}

Now you need the elevated privilages for sharepoint. So lets create a decorator for this one:

public class ElevatedPrivilegesResourceRepository : IResourceRepository
{
    private readonly IResourceRepository _resourceRepository;

    public ElevatedPrivilegesResourceRepository(IResourceRepository resourceRepository)
    {
        _resourceRepository = resourceRepository;
    }

    public Resource GetByLoginname(string loginname)
    {
        var result = default(Resource);

        Microsoft.SharePoint.SPSecurity.RunWithElevatedPrivileges(
            () => result = _resourceRepository.GetByLoginname(loginname));

        return result;
    }
}

You will pass an instance of the first repository to this one and it will execute the query with sharepoint's elevated privilages.

var repo = new ElevatedPrivilegesResourceRepository(new ResourceRepository());

var resource = repo.GetByLoginname("foo");

There is one more change that I've made. I put the cretion of a resource into a factory method of this class:

public class Resource
{
    public string RessourceUID { get; set; }
    public string RessourceName { get; set; }
    public List<Team> Teams { get; set; }
    public string Loginname { get; set; }

    public static Resource From(SqlDataReader reader)
    {
        var resource = new Resource
        {
            RessourceUID = SqlReaderHelper.GetValue<string>(dr, "ResourceUID"),
            RessourceName = SqlReaderHelper.GetValue<string>(dr, "ResourceName"),
            Loginname = SqlReaderHelper.GetValue<string>(dr, "UserClaimsAccount")
        }

        return resource;
    }
}

and I also modified the query itself:

using (var reader = cmd.ExecuteReader())
{
    while (reader.Read())
    {
        var resource = Resource.From(reader);
        if (resource != null)
        {
            resource.Teams = GetResourceTeams(resource);
        }
        return resource;
    }
    return null;
}

As you are looking for only one resource the reader won't proceed if there is none so you don't have to check whether it has any rows. You can just return the result after the first one.


Select Items from List using Array Is there a better way to write this procedure?

    public List<IDenomination> GetInventory(int[] denominations)
    {
        var inventory = new List<IDenomination>();

        foreach (var denomination in denominations)
        {
            foreach (var item in Account)
            {
                if (item.Currency == denomination)
                {
                    inventory.Add(item);
                }
            }
        }

        return inventory;
    }

You could use some linq to do this fairly easily. Converting the array to a HashSet will allow Contains to operate faster

public List<IDenomination> GetInventory(int[] denominations)
{
    var denoms = new HashSet<int>(denominations);
    return Account.Where( a => denoms.Contains(a.Currency) ).ToList();
}

This assumes that Account : IDenomination though, as does your original code. If that were not the case, you would use a .Select() prior to .ToList() in order to select the IDenomination from Account.


Game of Life with NumPy I started this exercise with NumPy with a goal to find neighbors and return the new matrix. I want to get your feedback. Here's an example from this website. It looks like it's \$O(N^2)\$, and I'm adding a internal loop to look around neighbors.

import numpy as np
import pprint

world = np.array([[0, 0, 0, 0, 0],
                  [0, 0, 1, 0, 0],
                  [0, 0, 1, 0, 0],
                  [0, 0, 1, 0, 0],
                  [0, 0, 0, 0, 0]])

pprint.pprint(world)
size = world.shape[0]

def next_state(world):
    """

    :param world:
    :return:
    """
    size = world.shape[0]
    neighbors = np.zeros(shape=(size, size), dtype=int)
    new_world = np.zeros(shape=(size, size), dtype=int)
    neighbor_count = 0
    # Ignore edges: start xrange: in 1
    for rows in xrange(1, size - 1):
        for cols in xrange(1, size - 1):
            # Check neighbors
            for i in [-1, 0, 1]:
                for j in [-1, 0, 1]:
                    # Condition to not count existing cell.
                    if rows + i != rows or cols + j != cols:
                        neighbor_count += world[rows + i][cols + j]
                        neighbors[rows][cols] = neighbor_count

            if neighbors[rows][cols] == 3 or (world[rows][cols] == 1 and neighbors[rows][cols] == 2):
                new_world[rows][cols] = 1
            else:
                new_world[rows][cols] = 0
            neighbor_count = 0

    pprint.pprint(neighbors)
    return new_world

print next_state(world)

That next_state function creates two brand new numpy array. Creating numpy array is slow. Should just update an existing numpy array.
Can divide the code into two classes. One for world, the other for the engine. World can have the world array and visualization. Engine can have the neighbor array.
Actually the neighbor array can be much smaller than the world if we update the world from left to right.
Python loop over each element (the row and col loops) is much slower than numpy's method. Can vectorize counting of neighbor by shifting the world and add to neighbor:

.

neighbor = np.zeros(world.shape, dtype=int)
neighbor[1:] += world[:-1]  # North
neighbor[:-1] += world[1:]  # South
neighbor[:,1:] += world[:,:-1]  # West
neighbor[:,:-1] += world[:,1:]  # East

neighbor[1:,1:] += world[:-1,:-1]  # NW
neighbor[1:,1:] += world[:-1,:-1]  # NE

Draw animation of world with matplotlib:

import numpy as np
import matplotlib.pyplot as plt

class World(object):
    def __init__(self, shape, random=True, dtype=np.int8):
        if random:
            self.data = np.random.randint(0, 2, size=shape, dtype=dtype)
        else:
            self.data = np.zeros(shape, dtype=dtype)
        self.shape = self.data.shape
        self.dtype = dtype
        self._engine = Engine(self)

        self.step = 0

    def animate(self):
        return Animate(self).animate()

    def __str__(self):
        # probably can make a nicer text output here.
        return self.data.__str__()

class Animate(object):
    def __init__(self, world):
        self.world = world
        self.im = None

    def animate(self):
        while (True):
            if self.world.step == 0:
                plt.ion()
                self.im = plt.imshow(self.world.data,vmin=0,vmax=2,
                                     cmap=plt.cm.gray)
            else:
                self.im.set_data(self.world.data)

            self.world.step += 1
            self.world._engine.next_state()
            plt.pause(0.01)
            yield self.world

class Engine(object):
    def __init__(self, world, dtype=np.int8):
        self._world = world
        self.shape = world.shape
        self.neighbor = np.zeros(world.shape, dtype=dtype)
        self._neighbor_id = self._make_neighbor_indices()

    def _make_neighbor_indices(self):
        # create a list of 2D indices that represents the neighbors of each
        # cell such that list[i] and list[7-i] represents the neighbor at
        # opposite directions. The neighbors are at North, NE, E, SE, S, SW,
        # W, NE directions.
        d = [slice(None), slice(1, None), slice(0, -1)]
        d2 = [
            (0, 1), (1, 1), (1, 0), (1, -1)
        ]
        out = [None for i in range(8)]
        for i, idx in enumerate(d2):
            x, y = idx
            out[i] = [d[x], d[y]]
            out[7 - i] = [d[-x], d[-y]]
        return out

    def _count_neighbors(self):
        self.neighbor[:, :] = 0  # reset neighbors
        # count #neighbors of each cell.
        w = self._world.data
        n_id = self._neighbor_id
        n = self.neighbor
        for i in range(8):
            n[n_id[i]] += w[n_id[7 - i]]

    def _update_world(self):
        w = self._world.data
        n = self.neighbor

        # The rules:
        #    cell        neighbor    cell's next state
        #    ---------   --------    -----------------
        # 1. live        < 2         dead
        # 2. live        2 or 3      live
        # 3. live        > 3         dead
        # 4. dead        3           live

        # Simplified rules:
        #    cell        neighbor    cell's next state
        #    ---------   --------    -----------------
        # 1. live        2           live
        # 2. live/dead   3           live
        # 3. Otherwise, dead.

        w &= (n == 2)  # alive if it was alive and has 2 neighbors
        w |= (n == 3)  # alive if it has 3 neighbors

    def next_state(self):
        self._count_neighbors()
        self._update_world()

def main():
    world = World((1000, 1000))

    for w in world.animate():
        pass

if __name__ == '__main__':
    main()


Builder design pattern pass object Good Morning

This is a builder design pattern .. the builder problem is duplication so

I want to know from an expert with explanation which is better on quality on this code:

(1)

public class Animal {

    private String animalName;
    private int animalAge;

    public static class Builder {
        private Animal animal;

        public Builder() {
            animal = new Animal();
        }

        public Builder getName(String animalName) {
            animal.animalName = animalName;
            return this;
        }

        public Builder getAge(int animalAge) {
            animal.animalAge = animalAge;
            return this;
        }

        public Animal build(){
            animal.build(this);
            return animal;
        }
    }

    public Animal(){ }

    public void build(Builder builder) {
        this.animalName = builder.animal.animalName;
        this.animalAge = builder.animal.animalAge;
    }

    public String getAnimalName() {
        return animalName;
    }

    public int getAnimalAge() {
        return animalAge;
    }
}

(2)

public class Animal {

    private String animalName;
    private int animalAge;

    public static class Builder{

        private String animalName;
        private int animalAge;

        public Builder getName(String animalName){
            this.animalName = animalName;
            return this;
        }

        public Builder getAge(int animalAge){
            this.animalAge = animalAge;
            return this;
        }

        public Animal build(){
            return new Animal(this);
        }

    }

    public Animal(Builder builder) {
        this.animalName = builder.animalName;
        this.animalAge = builder.animalAge;
    }

    public String getAnimalName() {
        return animalName;
    }

    public int getAnimalAge() {
        return animalAge;
    }
}

in first model i pass animal object and create an instance and then the builder fill the data and build the animal again .

in second model i duplicate all the animal instance variables

which is true ? and which not broke builder rules ?

What I've seen the most when it comes to builder patterns is that the object (in your case Animal) will get constructed in the build method of the builder. It is preferred to create instances of objects with all the data that the object should have, eg not creating invalid states of an object. This prevents you from having illegal state in your program which might produce bugs. By constructing your Animal as soon as you create a builder instance you have to make your Animal mutable which is not always what you want. For example, in some cases a class might have only final fields which can only be assigned via constructor.

In your case, constructing the Animal in the build method it would look something like this:

public class Animal {

    private String animalName;
    private int animalAge;

    public static class AnimalBuilder {
        private String animalName;
        private int animalAge;

        public AnimalBuilder() {
        }

        public Builder withName(String animalName) {
            this.animalName = animalName;
            return this;
        }

        public Builder withAge(int animalAge) {
            this.animalAge = animalAge;
            return this;
        }

        public Animal build(){
            Animal animal = new Animal();
            animal.animalName = this.animalName;
            animal.animalAge = this.animalAge;
            return animal;
        }
    }

    public Animal(){ }

    public String getAnimalName() {
        return animalName;
    }

    public int getAnimalAge() {
        return animalAge;
    }
}

You can then use it

Animal animal = new AnimalBuilder()
    .withName("Terry")
    .withAge(5)
    .build();

You can also add a static method builder() to the Animal class which returns a new builder:

public class Animal {
    ...

    public static AnimalBuilder builder() {
        return new AnimalBuilder();
    }
}

Animal animal = Animal.builder()
    .withName("Terry")
    .withAge(5)
    .build();

Does this mean a lot of duplication? Unfortunately, yes, builder patterns need a lot of duplicate code. You can also look at Lombok's Builder annotation which will generate all the builder boilerplate code for you. In that case it will just be

@Builder
public class Animal {

    private String animalName;
    private int animalAge;

    public Animal(){ }

    public String getAnimalName() {
        return animalName;
    }

    public int getAnimalAge() {
        return animalAge;
    }
}
```


Reading a configuration file in c# The configuration file of my app is similar to,

<?xml version="1.0" encoding="utf-8" ?>
<Items>
      <Item Name="Coffee"
            Cost="10"
            Image="itemCoffee.png" />
      <Item Name="Tea"
            Cost="10"
            Image="itemTea.png" />
      <Item Name="Vada"
            Cost="10"
            Image="itemVada.png" />
  </Items>

Just trying to read the above small configuration file and I wrote this method.

public static class Configuration
{
    public static T DeSerialize<T>(string filePath)
    {
        if (!System.IO.File.Exists(filePath))
        {
            throw new System.IO.FileNotFoundException(filePath);
        }

        var serializer = new System.Xml.Serialization.XmlSerializer(T);
        return (T)serializer.Deserialize(new FileStream(filePath, FileMode.Open));
    }
}

Where should I use using in this code? (Because, I never ever dispose the new FileStream that I wrote in this method)
Is this an overkill for reading this simple xml file?

I would definitely wrap the FileStream object into a using clause like this:

using (Stream reader = new FileStream(filePath, FileMode.Open))
{
  var serializer = new System.Xml.Serialization.XmlSerializer(typeof(T));
  return (T)serializer.Deserialize(reader);
}

This way the Dispose method of the FileStream gets called immediately. Otherwise it will be delayed until the FileStream object gets garbage collected. This may not be a big problem in practice in simple applications where you read the configuration only once but is good practice to do it anyway.
Also note that you have to use XmlSerializer(typeof(T)) instead of XmlSerializer(T).
My opinion is that it is overkill to create a generic method if you have only one kind of configuration file in your app. This method might be hard to read for a maintainer who is not sufficiently comfortable with generics and it has no benefit in making your code DRY-er if you have only one type of configuration file.


Search and Replace (FreeCodeCamp intermediate algorithm scripting) I completed this challenge Search and Replace and passed the tests:

  Perform a search and replace on the sentence using the arguments provided and return the new sentence.

  First argument is the sentence to perform the search and replace on.

  Second argument is the word that you will be replacing (before).

  Third argument is what you will be replacing the second argument with (after).

  Note

  Preserve the case of the first character in the original word when you are replacing it. For example if you mean to replace the word "Book" with the word "dog", it should be replaced as "Dog".

  Example

  myReplace("Let us go to the store", "store", "mall") should return "Let us go to the mall".

I was hoping to simplify my code. Is there a more concise way to write this program that would end up being cleaner and faster than the current code?

function myReplace(str, before, after) {
  let newStr = str.split(' ');
  for (var a=0; a < str.length; a++) {
    if(before === newStr[a]) {
      str = str.replace(before, after);
    }
    if (before[0] === before[0].toUpperCase()) {
      var swap = after[0].toUpperCase() + after.slice(1);
      str = str.replace(before, swap)
    }
  }
  return str;
}

console.log(myReplace("A quick brown fox jumped over the lazy dog", "jumped", "leaped"));

This kind of test boils down the the familiarity of built-in JavaScript APIs. The naive way to go about this problem is to scan through the string and do the checks manually. However...

Pass a function as second argument of string.replace() and it will call that function for each match it encounters. The return value of this function becomes the replacement. So instead of manually scanning the string, you can let string.replace() do that heavy-lifting. Note that passing a string as first argument to replace only makes it run once, which is why the first argument is a RegExp with a g flag constructed from the string you want replaced.

Also, a minor suggestion. Instead of comparing the first letter with its upper case version to see if it is upper case, you can use regex.test() to see if a character matches a pattern that only matches uppercase. It's a bit shorter, if length is what you're after.

function myReplace(str, before, after) {
  return str.replace(new RegExp(before, 'g'), match => {
    return (/[A-Z]/).test(before[0]) ? `${after[0].toUpperCase()}${after.slice(1)}` : after
  })
}

console.log(myReplace("Let us go to the store", "store", "mall"))
console.log(myReplace("He is Sleeping on the couch", "Sleeping", "sitting"))
console.log(myReplace("This has a spellngi error", "spellngi", "spelling"))
console.log(myReplace("His name is Tom", "Tom", "john"))
console.log(myReplace("Let us get back to more Coding", "Coding", "algorithms"))
console.log(myReplace("foo foo foo foo bar foo", "foo", "baz"))


Simplifying constructing native SQL query with nested if-else's I've a method which is used to build a native sql query. I've 4 String builder as input and based on different conditions, I need to build my query.

Here is the code

private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
    StringBuilder condition3) {
if (StringUtils.isNotEmpty(condition.toString())) {
    query.append(ApplicationConstants.AND+"(");
    query.append("( " + condition + " )");
    if (StringUtils.isNotEmpty(condition2.toString())) {
        query.append(ApplicationConstants.AND);
        query.append("( " + condition2 + " )");
        if (StringUtils.isNotEmpty(condition3.toString())) {
            query.append(ApplicationConstants.AND);
            query.append("( " + condition3 + " )");
        }

    } else {
        if (StringUtils.isNotEmpty(condition3.toString())) {
            query.append(ApplicationConstants.AND);
            query.append("( " + condition3 + " )");
        }
    }
    query.append(" )");
} else {
    if (StringUtils.isNotEmpty(condition2.toString())) {
        query.append(ApplicationConstants.AND+"(");
        query.append("( " + condition2 + " )");
        if (StringUtils.isNotEmpty(condition3.toString())) {
            query.append(ApplicationConstants.AND);
            query.append("( " + condition3 + " )");
        }
        query.append(" )");
    } else {
        if (StringUtils.isNotEmpty(condition3.toString())) {
            query.append(ApplicationConstants.AND+"(");
            query.append("( " + condition3 + " )");
            query.append(" )");
        }
    }
}

}

When I run my Sonar report, it says the Cognitive complexity is higher for this method. Can I simplify this methods avoiding many if-else loops

Let's look for some patterns in your code:

First of all, we notice that you have three parameters with similar names. condition, condition2 and condition3. These names indicates a code smell, that we should use an array or list instead.

Taking one step at a time, we see this code is repeated multiple times:

if (StringUtils.isNotEmpty(condition.toString())) {
    query.append(ApplicationConstants.AND+"(");
    query.append("( " + condition + " )");

if (StringUtils.isNotEmpty(condition2.toString())) {
    query.append(ApplicationConstants.AND);
    query.append("( " + condition2 + " )");

if (StringUtils.isNotEmpty(condition3.toString())) {
    query.append(ApplicationConstants.AND);
    query.append("( " + condition3 + " )");

if (StringUtils.isNotEmpty(condition3.toString())) {
    query.append(ApplicationConstants.AND);
    query.append("( " + condition3 + " )");

if (StringUtils.isNotEmpty(condition2.toString())) {
    query.append(ApplicationConstants.AND+"(");
    query.append("( " + condition2 + " )");

if (StringUtils.isNotEmpty(condition3.toString())) {
    query.append(ApplicationConstants.AND);
    query.append("( " + condition3 + " )");

if (StringUtils.isNotEmpty(condition3.toString())) {
    query.append(ApplicationConstants.AND+"(");
    query.append("( " + condition3 + " )");

This indicates that we can extract a method. (We can also notice that the "(" should be added for the first true if-statement, this is important but we will ignore this for now and get back to this later).

private boolean addParameter(StringBuilder query, StringBuilder condition) {
    if (StringUtils.isNotEmpty(condition.toString())) {
        query.append(ApplicationConstants.AND);
        query.append("( " + condition + " )");
        return true;
    }
    return false;
}

private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
    StringBuilder condition3) {
if (addParameter(condition)) {
    if (addParameter(condition2)) {
        if (addParameter(condition3)) {
            // we don't actually care about the result here
        }
    } else {
        if (addParameter(condition3)) {
            // not here either
        }
    }
    query.append(" )");
} else {
    if (addParameter(condition2)) {
        if (addParameter(condition3)) {
            // again, don't care about result
        }
        query.append(" )");
    } else {
        if (addParameter(condition3)) {
            // yeah ok, here we actually care...
            query.append(" )");
        }
    }
}

Now it's pretty clear that query.append(" )"); should always be done last, so that can be moved out and then we can avoid some if statements.

private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
    StringBuilder condition3) {
if (addParameter(condition)) {
    if (addParameter(condition2)) {
        addParameter(condition3);
    } else {
        addParameter(condition3);
    }
} else {
    if (addParameter(condition2)) {
        addParameter(condition3);
    } else {
        addParameter(condition3);
    }
}
query.append(" )");

And now we're doing the same in both some if's and else's, so that can be simplified:

private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
    StringBuilder condition3) {
if (addParameter(condition)) {
    addParameter(condition2);
    addParameter(condition3);
} else {
    addParameter(condition2);
    addParameter(condition3);
}
query.append(" )");

Once again we're doing the same thing:

private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
    StringBuilder condition3) {
addParameter(condition);
addParameter(condition2);
addParameter(condition3);
query.append(" )");

So we're essentially doing the same thing three times. This is where a loop would be handy.

private void appendConditions(StringBuilder query, StringBuilder... conditions) {
    for (StringBuilder condition : conditions) {
        addParameter(condition);
    }
    query.append(" )");

And now of course, we don't really need that separate addParameter method.

private void appendConditions(StringBuilder query, StringBuilder... conditions) {
    for (StringBuilder condition : conditions) {
        if (StringUtils.isNotEmpty(condition.toString())) {
            query.append(ApplicationConstants.AND);
            query.append("( " + condition + " )");
        }
    }
    query.append(" )");

Now, let's get back to the parenthesis: +"(" is added in the first true if-statement in your original code. So this, in connection with the closing " )" can be handled by for example keeping a boolean to check for if an if-statement has been true so far.

private void appendConditions(StringBuilder query, StringBuilder... conditions) {
    boolean conditionUsed = false;
    for (StringBuilder condition : conditions) {
        if (StringUtils.isNotEmpty(condition.toString())) {
            query.append(ApplicationConstants.AND);
            if (!conditionUsed) {
                conditionUsed = true;
                query.append(" (");
            }
            query.append("( " + condition + " )");
        }
    }
    if (conditionUsed) {
        query.append(" )");
    }

But when reflecting more about what it is that you do, we see that you have a collection of StringBuilders, you check each if it is not empty, then you join them together with ApplicationConstants.AND.

    String specialConditions = Arrays.stream(conditions)
        .map(StringBuilder::toString)
        .filter(StringUtils::isNotEmpty)
        .collect(Collectors.joining(ApplicationConstants.AND));
    if (StringUtils.isNotEmpty(specialConditions)) {
        query.append(" (");
        query.append(specialConditions);
        query.append(" )");
    }

Voilà!


Controlling parallel jobs with condition_variable I'm trying to set up a system where I can have an arbitrary number of parallel tasks, all of which wait for a go signal, do their work, and then stop while a manager collects the results, prepares for the next batch of work, and so on.

The easy way I know to do this is to launch a packaged_task for each job, for each iteration. However, I have got the impression that launching threads (and more so with the higher abstractions) have some considerable overhead, which I would want to avoid. My limited testing of those other options confirm that belief.

The best I've come up with so far is to have threads that are kept alive, and loop based on signals from condition_variable entities.

std::mutex mx;
std::condition_variable global;
bool start = false;
bool ready = false;
std::map<int, std::atomic<bool>> status;
using namespace std;
void work(int n) {
    while (true) {
        {
            unique_lock<mutex> lock{mx};
            //std::cout<<"Job #"<<n<<" waiting for ready signal.\n";
            while (!ready) { global.wait(lock); }
        } {
            unique_lock<mutex> lock{mx};
            status[n] = false;
            //std::cout<<"Job #"<<n<<" ready, waiting for start signal.\n";
            while (!start) { global.wait(lock); }
        }
        this_thread::sleep_for(chrono::milliseconds(10)); // simulate work
        status[n] = true;
    }
}
int main(int argc, char ** argv) {
    const auto num_jobs = (argc<=1? 1000:atoi(argv[1]));
    vector<thread> jobs;
    for (auto n=1;n<=num_jobs;n++) { jobs.emplace_back(work, n); }
    while (true) {
        cin.ignore(); {
            unique_lock<mutex> lock{mx};
            start = false;
            ready = true;
            status.clear();
            global.notify_all();
        } cin.ignore(); {
            unique_lock<mutex> lock{mx};
            ready = false;
            start = true;
            global.notify_all();
        }
        while (!all_of(status.begin(), status.end(),   // Wait for all jobs
               [](auto&& t){ return t.second.load(); }))
        { /*this_thread::sleep_for(chrono::seconds(1));*/ }
    }
    for (auto& job : jobs) { job.join(); }
}

I tried to compact the code, but tell me if it's too obfuscated. What the program does is to create some arbitrary number of workers, which wait for the launch signal from cin and then perform the simulated work.

I had to create two stages for the task to possibly be in, in a ready-set-go mechanic where the thread lands in a waiting clause after completing its job. My first attempt was to use just one wait for the task, but I can't figure out a a way to disable the single flag before some tasks are looping back around and then would start a second iteration immediately.

So basically what I'm asking: is this a good approach to the problem discussed in the first paragraph? If yes, can it be made better? Surely this is a well trodden issue for programmers trying to do parallel optimization, what is the conventional way to deal with my scenario?

Overall
Unfortunately your code is broken.
The main thread can potentially call notify_all() twice before any of the threads are released from the first call wait(lock). Say I hit return several time very quickly.
It's even possible for the main thread to call notify_all() before any of  the children call wait(lock) the first time (what happens if I pipe a file into standard in).
If you want all threads and the main to wait and sync at a common point you must explicitly create what is called a sync barrier. This is usually done by having a variable that counts the threads that have reached the sync point and only allowing progress after all threads have reached it and decremented the counter to the point where it reaches 0.
To avoid confusion you will also need to use a separate condition_variable for sync and start points.
Finally this code will never finish.
for (auto& job : jobs) { job.join(); }

Waits for all the threads to finish. But none of the jobs ever exit the callback function work().
Note: you can't just remove this line. A thread who's destructor is called before the thread of execution finishes its work will generate an exception that causes the application to quit. So you should call join() on all threads to verify that the thread of execution has completed.
Design.
I understand the need to force all the children thread to wait at an initial sync point. They need to verify that the main thread has successfully initialized all the global state.
But after that each worker thread does not need to wait at a sync point (unless there is an application requirement to do so). It just needs to wait for work to become available grab it then do it. Once the work is done it is usual for a thread to check if there is more work available (as creating threads is rather expensive you usually don't want to create and destroy threads regularly). If no work is available then make it wait on a condition variable until there is work.
All threads should periodically check to see if they are still needed and expire when they are no longer required. Thus allowing the main thread to exit.
Simple Code Review
Global variables a re a bad idea.
std::mutex mx;
std::condition_variable global;
bool start = false;
bool ready = false;
std::map<int, std::atomic<bool>> status;

wrap all this up in a class that you can then use locally inside a function.
Don't do this.
using namespace std;

If your code is more than 10 lines long you are going to cause issues sooner or later. See: Why is “using namespace std;” considered bad practice?
Threaded job Queue.
The following is a generalized job queue.
Now I read in your question that you need to have work done in synchronized bursts. This class does not do that but with some small adjustments that can be done.
void JobQueue::jobWorker()
{
    while(true)
    {
        auto job = getWork();
        if (job.execute())
        {
            // Indicate a thread should exit by returning true from execute.
            break;
        }
    }
}

class JobQueue
{
    private:
       void jobWorker();
    public:

        // Both addWork()/getWork() modify internal state.
        // So calls must be synchronized via locks.
        template<typename F>
        void addWork(F&& work); // Adds a job to an internal queue.
        Job getWork();          // If no work is available will cause the
                                // thread to wait (on a condition variable)
                                // until there is work available.

         // Constructor initializes all internal state.
         // Then creates `count` threads of execution that call jobWorker.
         JobQueue(int count);

         // The destructor waits for all work (added via addWork())
         // to be finished.
         //
         // This means it must:
         // 1) wait for the work queue to drain.
         // 2) all threads must be given a job that will return true.
         //    This will allow all the worker threads to exit.
         // 3) Wait for all worker threads by calling join() on them.
         ~JobQueue();
};

Then main is simplified too:
int main()
{
    JobQueue    jobQueue(4);  // not much point it making this number
                              // larger than the number of cores available
                              // on your machine. Actually you can get a default
                              // value that is the number of cores.

    // Call addWork() as many times as required.
    for(int loop = 0;loop < 1'000'000;++loop) {
        jobQueue.addWork([](){std::cout << "Some Work\n";sleep(5);});
    }
}


"Heat spot" image generation I wrote a function that generates an image like:

The function is as follows:

    /** Draws a texture wherein are spots of "heat" where a pixel's spotColor is of highest alpha, fading as distance from heat
     * spots increases.
     * @param width
     * @param height
     * @param spotColor The color of the spots, ignores alpha.
     * @param density
     * @return */
    public static Texture drawHeatSpots (int width, int height, Color spotColor, Magnitude density) {
        Pixmap p = new Pixmap(width, height, Format.RGBA8888);

        int spotCount;
        if (density == Magnitude.high)
            spotCount = width * height / 5000;
        else if (density == Magnitude.medium)
            spotCount = width * height / 6500;
        else if (density == Magnitude.low)
            spotCount = width * height / 8000;
        else
            throw new IllegalArgumentException(density.name() + " is an invalide Magnitude");

        Array<Vector2> spots = new Array<>();
        boolean next;

        for (int i = 0; i < spotCount; i++) {
            next = false;
            while (!next) {
                Vector2 v = new Vector2(MathUtils.random(width), MathUtils.random(height));
                if (!spots.contains(v, false)) {
                    spots.add(v);
                    next = true;
                }
            }
        }

        Vector2 v = new Vector2();
        Color c = new Color(spotColor);

        for (int x = 0; x < width; x++) {
            for (int y = 0; y < height; y++) {
                v.set(x, y);
                if (spots.contains(v, false)) {
                    c.a = 1;
                    p.drawPixel(x, y, Color.rgba8888(c));
                    continue;
                }
                c.a = calcAlpha(v, spots);
                p.drawPixel(x, y, Color.rgba8888(c));
            }
        }

        return new Texture(p);
    }

    private static float calcAlpha (Vector2 pos, Array<Vector2> spots) {
        float a = 0;
        for (Vector2 v : spots) {
            a += 1f / Math.sqrt(Math.pow(pos.x - v.x, 2) + Math.pow(pos.y - v.y, 2));
        }
        if (a > 1) a = 1;
        if (a < 0) a = 0;

        return a;
    }
}

In the picture, the magnitude is at high and it takes roughly 3 seconds to generate the image. At low, it takes about 2 seconds. Preferably, I would like the generation to take less than 1 second on high (without decreasing the number of spots).

Is there any way I can speed up the generation of this image? Or is this the best and fastest way to get an image similar to the one?

Time Complexity

Although you should always profile when concerned with performance. I'll go out on a limb here and make an educated guess.

    for (int x = 0; x < width; x++) {
        for (int y = 0; y < height; y++) {
            v.set(x, y);
            if (spots.contains(v, false)) {
                c.a = 1;
                p.drawPixel(x, y, Color.rgba8888(c));
                continue;
            }
            c.a = calcAlpha(v, spots);
            p.drawPixel(x, y, Color.rgba8888(c));
        }
    }

Here spots.contains and calcAlpha are both \$\mathcal{O}(n)\$ where \$n\$ is the number of spots. This means that for example the inner most loop of calcAlpha:

    for (Vector2 v : spots) {
        a += 1f / Math.sqrt(Math.pow(pos.x - v.x, 2) + Math.pow(pos.y - v.y, 2));
    }

is executed \$w \cdot h \cdot \frac{w\cdot h}{5000} = \frac{(w\cdot h)^2}{5000} = 169869312\$ times for 1280x720 (which I gather from your screenshot). Notice the square on \$w\cdot h\$? That's your problem.

How to solve it? Well you can use a HashSet for spots, this reduces spot.contains to \$\mathcal{O}(1)\$ amortized. But you're still left with calcAlpha.

You need to come up with a smarter algorithm that doesn't have to iterate over all the spots for all pixels.

Cache Efficiency

The CPU has a little thing called a "pre-fetcher". Simply put when you ask for memory address x it will also make sure that x+1 and x-1 are in the CPU cache. This means that linear access of memory is much faster than any other way of addressing memory. Particularly, "randomly" accessing memory is bound to give you a metric f-ton of cache misses and your performance will be a proper charlie-foxtrot.

I am not aware of any graphics library or hardware that stores images in column-major mode. This means that you should always process images, row-by-row. Meaning that the x coordinate should always be the inner most loop.

I.e. this:

for (int x = 0; x < width; x++) {
    for (int y = 0; y < height; y++) {

should be:

for (int y = 0; y < height; y++) {
    for (int x = 0; x < width; x++) {

for maximal performance (note the swapped order of x and y).

Improved Algorithm

The key idea to note for improving the algorithm is that each of the spots has the same influence on it's neighbourhood. I.e. all the distance calculations for each dot are the same, just with a different offset. This means that we can pre-calculate the results once and just blit.

Define radius as the radius in pixels from a spot where you cut off it's influence on the image.

Below is pseudocode to show the idea. It may contain bugs, and there is definitely some assembly required. Like checking image bounds and using a 1D dense array instead of a 2D nested array (which is done for clarity):

// Prepare alpha-map.
// Time: O(radius^2)
float[][] alphaMap = new float[2*radius+1][2*radius+1];
for(int y = 0; y < 2*radius + 1; ++y){
    for(int x = 0; x < 2*radius +1; ++x){
        int dx = x - radius;
        int dy = y - radius;
        int hypot2 = dx*dx + dy*dy;
        if(dx == 0 && dy == 0){
            alphaMap[y][x] = 1.0f;
        }else if (hypot2 < radius*radius){
            alphaMap[y][x] = 1.0f / Math.sqrt(hypot2);
        }
    }
}

// Calculate alpha values for entire image at once
// Time: O((w*h)*radius^2)
float[][] alphaBuffer = new float[height][width];
for(spot : spots){
    int xMin = spot.x - radius;
    int xMax = spot.x + radius +1;
    int yMin = spot.y - radius;
    int yMax = spot.y + radius +1;
    for(int y = yMin; y < yMax; ++y){
        int dy = y - spot.y;
        for(int x = xMin; x < xMax; ++x){
            int dx = x - spot.x;
            alphaBuffer[y][x] += alphaMap[radius + dy][radius + dx];
        }
    }
}

Note that this has the same asymptotic time complexity as OP's own answer \$\mathcal{O}(whr^2)\$. But this computes the reciprocal of the hypotenuse only \$\pi r^2\$ times while OPs answer does it \$\frac{wh}{5000}\pi r^2\$ times. The calculation of the reciprocal of the hypotenuse is the most time consuming part in this algorithm and should dominate the execution time.

Note: The observant reader will have noticed that the alphaMap grid is symmetrical in all quadrants. This means you can reduce the number of calculations of the reciprocal of the hypotenuse to \$\frac{\pi r^2}{4}\$ but that's left as an exercise to the reader.


Akka streams: Bidirectional flow that completes the stage when one side is completed This is meant to be a bidirectional flow BidiFlow that works exactly as BidiFlow.identity, except that it completes the stage as soon as one of the Flows (in1 -> out1 or in2 -> out2) is completed.

This can be useful to automatically close a connection fully when one of the two sides is closed.

There are a few tricks to prevent object allocation.

import akka.stream.{ Outlet, Inlet, Attributes, BidiShape }
import akka.stream.stage._

object DisconnectBidiFlow {

  def apply[A, B]: GraphStage[BidiShape[A, A, B, B]] = Impl.asInstanceOf[GraphStage[BidiShape[A, A, B, B]]]

  private object Impl extends GraphStage[BidiShape[Any, Any, Any, Any]] {

    val in1: Inlet[Any] = Inlet("DisconnectBidiFlow.in1")
    val out1: Outlet[Any] = Outlet("DisconnectBidiFlow.out1")
    val in2: Inlet[Any] = Inlet("DisconnectBidiFlow.in2")
    val out2: Outlet[Any] = Outlet("DisconnectBidiFlow.out2")

    override def createLogic(inheritedAttributes: Attributes): GraphStageLogic = new GraphStageLogic(shape) {
      def map[T](in: Inlet[T], out: Outlet[T]) = {
        val handler = new InHandler with OutHandler {
          override def onPush(): Unit =
            push(out, grab(in))

          override def onPull(): Unit =
            pull(in)
        }
        setHandler(in, handler)
        setHandler(out, handler)
      }
      map(in1, out1)
      map(in2, out2)
    }

    override def shape = BidiShape(in1, out1, in2, out2)
  }
}

Questions:

Can/should the shape be a val
Suggestions for a better name.

It is not really a BidiFlow but just shaped like one. It is a GraphStage[BidiShape[A, A, B, B]]
Should the name contain the purpose (FullDisconnectBidiFlow?)

Maybe talk a bit about the "tricks to prevent object allocation."

Based on the current examples in the Akka docs, the shape should be override val. My understanding is that the accepted pattern when designing traits is to def your contract, and implement with val wherever possible.

As they say - naming is hard and often a matter of taste. The pattern reminds me of a fuse, or breaker - but fuse has a particular meaning in Akka, so that's probably not a great choice. Maybe BreakerFlow?


Count the number of neighbors I have a numpy array called contour of \$N\$ coordinates (x,y) of dimension (N, 2).
For each point in this table, I would like to create a square centered at that point and perform a test on the square formed.
My current code is slow, probably due to the for loop. Can the execution speed be improved?
My code:
def neighbour(point , mask  , n ): # Create the square around this point and count the number of neighbors.
      mask = mask[point[0] - int(n/2) : point[0] + int(n/2) + 1,point[1] - int(n/2):point[1] + int(n/2) + 1]
      return n**2 - np.count_nonzero(mask)

def max_neighbour(contour , mask=maske , n=ne): # Find the point with as many neighbors as possible
    t = np.zeros(len(contour)) # contour is the numpy array of dimension (2,N)
    for i in range(len(contour)):
        t[i] = neighbour(contour[i],mask,n)
    return contour[np.argmax(t)] # t contains the list of neighbors for each contour point.

Okay, so there are a lot of problems here. I think the best way to address them is in stages. I want to show you how to go through your own code critically and address problems in it as you write.
Before I begin, for the benefit of someone who will review your code, it is helpful to include all the imports you used so that the person doing the review can just copy your code into their environment and run it if they need to.
So a good first inclusion would be:
import numpy as np
My first stylistic point is that you really should conform to PEP8, it creates a standard that all Python programmers should follow and while some of its requirements can be a bit frustrating at times it is, for the most part, a very good style guideline.
def count_neighbours(point, mask, n):
    # Create the square around this point and count the number of neighbors.
    # Change 1: changed name from 'neighbour' to 'count neighbours'.
    #     It is often helpful to have expressive names for functions and
    #     variables. Since functions DO something I find verb-object phrases
    #     very intuitive when I'm not working on a project where function
    #     names are standardised. Someone looking at your code can ask
    #     "what does this function do?", answer: oh it "counts neighbours".
    # Change 2: fixed spacing in arguments. Look into PEP8 and a linter that
    #     will tell you when you place spaces in poor or unconventional
    #     locations.
    mask = mask[point[0] - int(n/2) : point[0] + int(n/2) + 1,point[1] - int(n/2):point[1] + int(n/2) + 1]
    return n**2 - np.count_nonzero(mask)
    # Change 3: made indent of count_neighbours function a multiple of four.
    #     Again, please follow PEP8 guidelines.

All my present changes are simply style related, now lets talk about this... thing:
mask = mask[point[0] - int(n/2) : point[0] + int(n/2) + 1,point[1] - int(n/2):point[1]
There's no way around the fact that this is quite ungainly. What can we do about it? The biggest problem, aesthetics aside, is that at first glance I have no idea what it's actually supposed to do. Here I approve very much that you added a comment to give some explantion as to what this function does, that comment is the only thing that made figuring out the intention easier.
Python actually attaches a string encountered directly after a function defintion as a docstring for that function. You can find style guidelines for writing doctrings here. Let's make that comment more explicit and put it in a docstring.
def count_neighbours(point, mask, n):
    """
    Count the neighbours of a point, where neighbours are other points in
    mask that are within a square of side length n, centered on point.

    If a copy of point appears in mask it is counted as a neighbour.

    Arguments:
    point -- the point on which the square will be centered.
    mask -- the set of points to check.
    n -- the side length of the square.
    """
    mask = mask[point[0] - int(n/2) : point[0] + int(n/2) + 1,point[1] - int(n/2):point[1] + int(n/2) + 1]
    return n**2 - np.count_nonzero(mask)

Here I have guessed at what is supposed to be contained in mask and written it in the docstring. I have made this assumption based on what the function is supposed to do, but based on the call in your other function: neighbour(contour[i], mask, n) where you have said that contour is in fact your list of points I suspect there may be some confusion at the variable naming stage.
I am going to give mask the revised name all_points in order to make it clearer what the argument is, and update the docstring accordingly. Now, we've finally arrived back at this
all_points = all_points[point[0] - int(n/2) : point[0] + int(n/2) + 1,point[1] - int(n/2):point[1].
How can I make this easier to follow? One obvious idea is that, since we seem to be concerned with a plane we can name some variables of interest:
px = point[0]  # x-value (first coordinate) of the point p
py = point[1]  # y-value (second coordinate) of the point p
offset = n/2  # half of square side length

And now finally, after some cleaning up, we actually realise that there is a deeper problem here: the confusion expression was not only poorly written, but logically flawed! Why are we slicing using the nearest integer to the boundaries of the square as indicies? This makes no sense. We are interested in comparing the bounds px-offset and px+offset to the contents of all_points, not to the indicies of all_points.
So, what we really want to know which points in all_points satisfy these inequalities:
neighbours_x = all_points[:, 0] >= px-offset
# is a point >= the lower bound in x?
neighbours_x &= all_points[:, 0] <= px+offset
# is a point <= the upper bound in x?
neighbours_y = all_points[:, 1] >= py-offset  # lower bound in y
neighbours_y &= all_points[:, 1] <= py+offset  # upper bound in y
neighbours = neighbours_x & neighbours_y  # neighbours in both axes

Go through each step of this process and print the output at each step using the example given at the end and see what each step does.
Finally we want to return the number of neighbours. Both of the below expressions work. The former is faster, but the latter might give you more intuition about what we're doing:
return neighbours.sum()
return all_points[neighbours].shape[0]

I also recommend you print all_points[neighbours] and see what the output is, again using the example at the end.
import numpy as np
from itertools import product

def count_neighbours(point, all_points, n):
    """
    Count the neighbours of a point, where neighbours are other points in
    all_points that are within a square of side length n, centered on point.

    Arguments:
    point -- the point on which the square will be centered.
    all_points -- the set of points to check.
    n -- the side length of the square.
    """
    px = point[0]  # x-value (first coordinate) of the point p
    py = point[1]  # y-value (second coordinate) of the point p
    offset = n/2  # half of square side length
    neighbours_x = all_points[:, 0] >= px-offset
    # is a point >= the lower bound in x?
    neighbours_x &= (all_points[:, 0] <= px+offset)
    # is a point <= the upper bound in x?
    neighbours_y = all_points[:, 1] >= py-offset  # lower bound in y
    neighbours_y &= all_points[:, 1] <= py+offset  # upper bound in y
    neighbours = neighbours_x & neighbours_y  # neighbours in both axes

    return neighbours.sum()

# example 1
X = np.array(list(product([-2,-1,0,1,2], [-2,-1,0,1,2])))
count_neighbours(X[0], X, 2)

# example 2
X = np.array(list(product([-2.1,-1,0.3,1.7,2], [-2.2,-1.8,0.1,1,2])))
count_neighbours(X[0], X, 4.7)

I hope this helps you identify logical errors in future. I think you should be able to review and refactor the other function on your own. Feel free to edit your question with the updated version later if you still want a second opinion one it is clearer what the function is doing and then we can address performance issues.


Django query filter on count in one to many relation I have the following models:

class Donation(models.Model):
    donation_amount = models.DecimalField(max_digits=9, decimal_places=2)
    donor_name = models.CharField(max_length=200)

class Charge(models.Model):
    donation = models.ForeignKey(Donation)
    error = models.TextField(null=True, blank=True)

And I have a complex query that should do one of the following:

  Get all donations or all donations which has at least one failed charge.

I came up with the following solution:

# get all donations with charge prefetched
all_donations = Donation.objects.all().prefetch_related('charge_set')

# sometimes filter for failed ones
if need_failed_charges:
  all_donations = all_donations.annotate(count=Count('charge')) \
            .filter(charge__error__isnull=False, count__gt=0)

Here I use the count to track the number of charges I have per donation, I filter them afterward to check if I have at least one.

I think there is a better way to express this, but as I'm still new to the django world I cannot see it.

What do you think?

It's easier to answer this kind of question if you give us more code. I would have preferred the question to contain (possibly cut-down) code for your models, not just a summary description of the models.
The comment for the error field says "can be null/blank" but your code only tests for NULL, not for blank. If the comment is correct, then your code is buggy; conversely, if the code is correct, then the comment is wrong.
The question says, "I filter them afterward to check if I have more than one" but the code says, count__gt=0, so it is actually filtering for more than zero failed charges. If the text of question is right then the code is buggy.
There's no need for the count annotation — if you add a filter on a one-to-many relation then you only get results where there is at least one record that matches the filter. (In SQL terms, Django's filters on one-to-many relations are implemented as inner joins.)

So all that's needed is:

# Donations with at least one failed charge.
Donation.objects.filter(charge__error__isnull=False)

You can check this by looking at the SQL that is generated by Django's ORM:

>>> print(Donation.objects.filter(charge__error__isnull=False).query)
SELECT "myapp_donation"."id" FROM "myapp_donation"
  INNER JOIN "myapp_charge"
  ON ( "myapp_donation"."id" = "myapp_charge"."donation_id" )
  WHERE "myapp_charge"."error" IS NOT NULL

(The count annotation would, however, be necessary if the requirement were to select donations with more than one failed charge, as stated in the question.)


Hello parallel world I'm introducing someone to JavaFX. It still feels relatively new and I don't take being someone's first exposure to something lightly. I want to know if there's anything I'm doing unconventionally, inefficiently, or just plain oddly in this code? Although it is relatively simple, If fresh eyes could discern something unaccounted or discern a should-be inclusion, I'd be remiss not to illuminate self before attempting to do so for another.

import javafx.application.Application;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.control.Tooltip;
import javafx.scene.layout.HBox;
import javafx.stage.Stage;

public class Hello extends Application {
    Scene scene;

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage stage) {
        TextField entry = new TextField();
        entry.setPromptText("Enter your name");

        Button hello = new Button("Greeting");
        hello.setOnAction(e -> greet(entry));
        hello.setDefaultButton(true);
        hello.setTooltip(new Tooltip("Click me!"));

        HBox layout = new HBox();
        layout.getChildren().addAll(entry, hello);
        layout.setPadding(new Insets(2, 12, 2, 12));

        scene = new Scene(layout);

        stage.setScene(scene);
        stage.setTitle("Welcome to JavaFX");
        stage.show();
        hello.requestFocus();
    }

    public void greet(TextField entry) {
        String name = entry.getText();
        Label label = name.isEmpty() ? new Label("Hello user!") : new Label("How's it going, " + name + "?");
        scene.setRoot(label);
    }
}

Pretty neat. I think this:

Label label = name.isEmpty() ? new Label("Hello user!") : new Label("How's it going, " + name + "?");

Could be neater like this:

Label label = name.isEmpty()
    ? new Label("Hello user!")
    : new Label("How's it going, " + name + "?");

Actually... what changes depending on the result name.isEmpty() isn't the label itself, but its content.

String greeting = name.isEmpty()
    ? "Hello user!"
    : "How's it going, " + name + "?";
Label label = new Label(greeting);

I'd probably rename label to greetingLabel, too.


Modifying list while iterating during Breadth-first search I am writing a function for implementing breadth-first search over a no. of nodes in a directed graph. The nodes are all numbers from 1-100. This is my implementation:

def BFS(start= 1, adj_dict= None):
    level= {}
    current= [start]
    visible= []
    lno= 0
    step= 0
    for ele in current:
        if ele not in level:
            level[ele]= lno
            visible.extend(adj_dict[ele])
        current.remove(ele)
        if current==[]:
            current= visible[:]
            visible= []
            lno+=1
    return level

This did not work correctly and stored only the start key in adj_dict. I ran the debugger and saw that the code exited for loop after first iteration.

Upon searching a little about the cause of this, I found this question- https://stackoverflow.com/questions/1207406/remove-items-from-a-list-while-iterating on SO. This was the cause of error. The answers to that question suggest that I create a new list somehow, either using list comprehension or using slicing but none of them will help in my case.

So, I thought of nesting the for loop inside a while loop to achieve the desired result. This is how it looks now:

while current:
    for ele in current:
        if ele not in level:
            level[ele]= lno
            visible.extend(adj_dict[ele])
        current.remove(ele)
        if current==[]:
            current= visible[:]
            visible= []
            lno+=1

This works now. What I want to ask is this a correct way to achieve the effect or am I just getting lucky and the code can break easily? Also is this the pythonic way?

I do not know what is considered pythonic, but when doing a BFS, it is preferable to use a while loop. Also, you can use the queue (or Queue in Python 2.7) module for more intuitive operations:

from queue import Queue

def BFS(start=1, adj_dict=None):
    queue = Queue()
    already_seen = set()
    level = {}

    # this is our "seed" at starting level = 0
    queue.put_nowait((start, 0))

    while not queue.empty():
        # get the topmost element of the queue
        curr_node, curr_level = queue.get_nowait()

        # make sure not to process element more than once
        if curr_node not in already_seen:
            already_seen.add(curr_node)
            level[curr_node] = curr_level

            # add all of the current node's neighbors to the queue
            for neighbor_node in adj_dict[curr_node]:
                queue.put_nowait((neighbor_node, curr_level + 1))
    return level


Simple parser using Flex and C++ This is an alternative parser based on the specifications from this question.  Briefly stated, the input file is a text file which has at least 33 fields separated by semicolons.
If the fourth field begins with either a T or an E, the line is valid and a subset of it written to the output file.  Specifically, fields as numbered from \$0\$, should be output in this order: \$ \{0, 2, 3, 4, 5, 6, 10, 9, 11, 7, 32\}\$, each separated by a comma.  All other fields are discarded.
One of the other answers there suggested that one could use a Flex-based parser instead. My own efforts were not faster, but I'm hoping that someone can review this and show me how to extract more speed from this version.
lexer.l
%{
#include <iostream>
#include <fstream>
#include <vector>
#include <string>
#include <algorithm>
#include <experimental/iterator>
#include <iterator>
#undef YY_DECL
#define YY_DECL int FileLexer::yylex()

class FileLexer : public yyFlexLexer {
public:
    FileLexer(std::istream& in, std::ostream& out) :
        yyFlexLexer{&in, &out},
        out{out}
    {}
    using FlexLexer::yylex;
    /// the yylex function is automatically created by Flex.
    virtual int yylex();
private:
    /// pointer to the current value
    std::vector<std::string> vec;
    std::ostream& out;
    unsigned fieldcount{0};
    bool valid{true};
};

%}
%option warn nodefault batch noyywrap c++
%option yyclass="FileLexer"

FIELD  [^;\n]*
DELIM   ;
%%

{DELIM} { }
\n      {
            if (valid && fieldcount >= 33) {
                std::copy(vec.begin(), vec.end(), std::experimental::make_ostream_joiner(out, ","));
                out << '\n';
            }
            vec.clear();
            fieldcount = 0;
            valid = true;
            return 1;
        }
{FIELD} {
            if (valid) {
                switch (fieldcount++) {
                    case 0:
                    case 1:
                    case 4:
                    case 5:
                    case 6:
                    case 7:
                    case 9:
                    case 32:
                        vec.push_back(yytext);
                        break;
                    case 3:
                        if (yytext[0] == 'E' || yytext[0] == 'T') {
                            vec.push_back(yytext);
                            valid = true;
                        } else {
                            valid = false;
                        }
                        break;
                    case 10:
                        {
                            auto n{vec.size()};
                            vec.push_back(yytext);
                            std::iter_swap(vec.begin()+n, vec.begin()+n-2);
                        }
                        break;
                    case 11:
                        {
                            auto n{vec.size()};
                            vec.push_back(yytext);
                            std::iter_swap(vec.begin()+n, vec.begin()+n-1);
                        }
                        break;

                }
            }
        }
%%
int main(int argc, char *argv[]) {
    if (argc >= 3) {
        std::ifstream in{argv[1]};
        std::ofstream out{argv[2]};
        FileLexer lexer{in, out};
        while (lexer.yylex() != 0)
        {}
    }
}

Compile with:
flex -o parsefile.cpp lexer.l
g++ -O2 -std=gnu++17 parsefile.cpp -o parsefile

This works, but it is slow (2.165 s) on my machine, with the same million-line input file as mentioned in my answer to the other question.
I tried it a few different ways, but I was unable to get a version that was faster than the PHP code in the other question.  The switch statement logic is arguably a bit overly clever and stores only the needed fields in the desired order, but the speed was about the same as the straightforward implementation.
If it matters, I'm using gcc version 10.1 and flex 2.6.4 on a 64-bit Linux machine.

Overview
There is an issue (Bug) here in that yytext points at the beginning of the lexeme. But the lexeme is not null ('\0') terminated. You need to pass a length if you want to pass the current token into vec
   vec.push_back(yytext);

   // should be:
   vec.emplace_back(yytext, yytext + yylen);

You had a bug in your call to the underlying base class.
    FileLexer(std::istream& in, std::ostream& out) :
        yyFlexLexer{&in, &out},
        out{out}
    {}

Sorry I fixed it before any answers. But you need to pass the address of the streams to yyFlexLexer.

Normally I would return a value for each lexeme (and move any complex processing into methods of FileLexer. BUT this is such a simple class I don't see any issue with your current implementation of putting all the code in the lexer directly (though I would remove the return 1; from the end of line marker to make it consistent with field processing).

This seems to be correct.
std::copy(vec.begin(), vec.end(), std::experimental::make_ostream_joiner(out, ","));
out << '\n';

But it is not obvious how it achieves it. Would be nice to have a comment that points out that field 10/11 are not added to the end but rather to a point not at the end of the end of the vector.


Console Worker Follow-Up A few hours ago I posted a similar question but this is a follow-up, I have revised the code and added and removed certain lines of code and hopefully improved it a bit, I just brought it here for a final cleaning, can anyone notice anything than can be improved even more? Some of the code has been totally changed such as the way I wrote the uptime string

var consoleTitleString = string.Format("{0} / {1}, {2}, and {3}",
                    projectName,
                    serverUptime.Days + " day" + (serverUptime.Days != 1 ? "s" : ""),
                    serverUptime.Hours + " hour" + (serverUptime.Hours != 1 ? "s" : ""),
                    (StaticSettings.includeSecondsInUptimeString) ? serverUptime.Minutes +
                    " minute" + (serverUptime.Minutes != 1 ? "s" : "") + " and " +
                    serverUptime.Seconds + " second" + (serverUptime.Seconds != 1 ? "s" : "") :
                    serverUptime.Minutes + " and minute" + (serverUptime.Minutes != 1 ? "s" : "")
                    );

I feel its a bit squeezed in, is there a way to make it a bit more clean without the need of an if statement?

Just a few of the improvements I've made

Removed field comments
Improved long-lined code
Removed useless code

using Faze.Other.App;
using log4net;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace Faze.Other.Util.Console
{
    class ConsoleWorker : IDisposable
    {
        private readonly Timer consoleWorkerTimer;
        private readonly int consoleWorkerInterval;
        private readonly ILog classLogger;

        public ConsoleWorker()
        {
            consoleWorkerInterval = 1000;
            classLogger = LogManager.GetLogger(typeof(ConsoleWorker));
            consoleWorkerTimer = new Timer(timerElapsed, null, TimeSpan.FromMilliseconds(consoleWorkerInterval), TimeSpan.FromMilliseconds(consoleWorkerInterval));
        }

        /// <summary>
        /// Handles the timers callback once elapsed.
        /// </summary>
        /// <param name="timerObject">ConsoleWorker timer's object.</param>
        private void timerElapsed(object timerObject)
        {
            var serverInfo = Faze.getServer().getServerInformation();

            if (serverInfo.hasStarted())
            {
                if (StaticSettings.advancedLoggingWhenDebugging && Debugger.IsAttached)
                    classLogger.Info("ConsoleWorker has elapsed -> " + DateTime.Now.ToLongTimeString());

                TimeSpan serverUptime = DateTime.Now - serverInfo.getStartedTime();

                var projectName = serverInfo.getProjectName();

                var consoleTitleString = string.Format("{0} / {1}, {2}, and {3}",
                    projectName,
                    serverUptime.Days + " day" + (serverUptime.Days != 1 ? "s" : ""),
                    serverUptime.Hours + " hour" + (serverUptime.Hours != 1 ? "s" : ""),
                    (StaticSettings.includeSecondsInUptimeString) ? serverUptime.Minutes +
                    " minute" + (serverUptime.Minutes != 1 ? "s" : "") + " and " +
                    serverUptime.Seconds + " second" + (serverUptime.Seconds != 1 ? "s" : "") :
                    serverUptime.Minutes + " and minute" + (serverUptime.Minutes != 1 ? "s" : "")
                    );

                System.Console.Title = consoleTitleString;
            }
        }

        public void Dispose()
        {
            consoleWorkerTimer.Dispose();
        }
    }
}

One final thing, Someone on the old post said use CamalCase because I would seem like I'm coding JavaScript otherwise, is this a do do and a don't don't go against thing or is that just his personal preference?

I still find the string creation awful. If you cannot use C# 6 the I suggest the StringBuilder:

var consoleTitle = new StringBuilder()
    .Append(projectName).Append(" / ")
    .Append(serverUptime.Days).Append(" day".Pluralize(serverUptime.Days))
    .Append(serverUptime.Hours).Append(" hour".Pluralize(serverUptime.Hours))
    // ...
    .ToString();

and an extension method to get rid of conditionals:

static class StringExtensions
{
    public static string Pluralize(this string text, int value)
    {
        return (value != 1 ? text + "s" : text)
    }
}


Python Reverse the binary equivalent of input and output the integer equivalent of the reverse binary Am supposed to capture user input as an integer, convert to a binary, reverse the binary equivalent and convert it to an integer.Am getting the right output but someone says the solution is wrong. Where is the problem?

x  = 0

while True:

    try:
        x = int(raw_input('input a decimal number \t'))

        if x in xrange(1,1000000001):

            y = bin(x)

            rev = y[2:]

        print("the reverse binary soln for the above is %d") %(int('0b'+rev[::-1],2))
            break

    except ValueError:
        print("Please input an integer, that's not an integer")
        continue

x  = 0

There is no point in doing this. You just replace it anyways

while True:

    try:
        x = int(raw_input('input a decimal number \t'))

        if x in xrange(1,1000000001):

Probably better to use if 1 <= x <= 100000000001: although I'm really not sure why you are doing this check. Also, you should probably explain to the user that you've reject the number.

            y = bin(x)

            rev = y[2:]

I'd use reversed = bin(x)[:1::-1] rather then splitting it out across the tree lines.

        print("the reverse binary soln for the above is %d") %(int('0b'+rev[::-1],2))

I'd convert the number before the print to seperate output from the actual math.

            break

    except ValueError:
        print("Please input an integer, that's not an integer")
        continue

This continue does nothing.


Refining AI movement logic I have the below class which moves the AI towards the given plant, this works well however it feels really messy.

Any input as to a better way to lay out the logic would be really grateful, the logic follows the idea that we will take the x and y positions of the AI from the plants position, if the values are positive we add 30 and if it is negative we take away 30

If you need any more explaining of the logic let me know.

private void movePosistion(Plant p) {
        /*
         * set which direction to move,the number generated relates to the
         * direction as below:
         *      1   2   3
         *      4       5
         *      6   7   8
         */
        int xdiff = p.getXpos() - xpos;
        int ydiff = p.getYpos() - ypos;
        if (xdiff > 0){
            if (ydiff > 0){
                //8
                xpos += 30;
                ypos += 30;
            }else if(ydiff < 0){
                //3
                xpos += 30;
                ypos -= 30;
            }else{
                //5
                xpos += 30;
            }
        }else if(xdiff < 0){
            if (xdiff > 0){
                //6
                xpos -= 30;
                ypos += 30;
            }else if(xdiff < 0){
                //1
                xpos -= 30;
                ypos -= 30;
            }else{
                //4
                xpos -= 30;
            }
        }else{
            if (ydiff < 0){
                //7
                ypos -= 30;
            }else{
                //2
                ypos += 30;
            }
        }

        if (xpos > 720)
            xpos = 1;
        if (ypos > 720)
            ypos = 1;
        if (xpos < 1)
            xpos = 720;
        if (ypos < 1)
            ypos = 720;
    }

Here is my refactoring:

private void movePosition(Plant p) {
    xpos += Integer.signum(p.getXpos() - xpos) * DELTA_X;
    ypos += Integer.signum(p.getYpos() - ypos) * DELTA_Y;

    xpos = Math.floorMod(xpos, MAX_X);
    ypos = Math.floorMod(ypos, MAX_Y);
}

1. Signum

signum implements the sign function, which gives -1, 0 or 1 for negative integers, zero and positive integers respectively. It encodes the original logic in a very short and readable expression.
The sign is multiplied by the appropriate amount of units (constants are not detailed in the code, btw).

I have nothing against decision tables in principle (see rolfl's answer), but I don't think this is necesary here.
In his answer, palacsint cited "Code Complete". I can do that too!

From Code Complete 2nd Edition, Chapter 19: General Control Issues, page 431:

  Use decision tables to replace complicated conditions

  Sometimes you have a complicated test involving several variables. [...]

... and sometimes you don't ;-)

2. Modulus

The wrap-around behaviour of the original code can be achieved with a modulus operation. Note that you cannot just use the % operator in Java because it computes the remainder, which can be negative when your position goes below zero. The floorMod operation actually computes modular arithmetic.

Now, you might think: this is wrong, the original code does not work like this!
Yes, but let me explain:

First, OP's coordinates range from 1 to 720 (both inclusive). I have an issue with this and I deliberately changed the approach here. The reason is that, in the original code, instead of using a coordinate space that have the origin at (0,0), the origin is translated at (1,1).

Most of the time, you end up having to add or substract 1 to you operations. That can eventually lead to off-by-one errors. If coordinates are from 0 to 719, however, the wrap-around logic is simply given by the modulus operation.
"But the original behavior is not like a modulus!", you might say. Why do you say this? because, suppose x is 710, and then I add 30: with modulus, I goes back to 20, whereas using OP's code, I would have 1 because when we are out of bounds, we go back to the minimal (or maximal) position.

To that, I reply that you never are at position 710, but only at 0, 30, 60, ..., 690. At least, this is what I understand from OP's code, where the object seems to move on a grid, and not freely around. I suppose the object is always located initially at a multiple of 30, and then can only move by 30 units.

If I am wrong, then (1) sorry, and (2) yes, the modulus is not exactly the good answer; you might better use the boundPos function from rolfl.


Managing inherited dependency properties in WPF Exposing Inherited Members

I have a WPF Custom Control library (ContentToggleButton) and an app (spec) to test it. The Custom Controls, which derive from Presentation Framework elements, expose inherited properties as well as new properties to the consumer.

This means that the consuming app depends on the Framework Elements as well as the Custom Controls as shown by the four dependencies from the top and bottom of MainWindow in the following diagram.

Reimplementing Inherited Members

The references were due to the IsChecked dependency property of the ToggleButton base class of the ContentToggle object and the Click event of the ButtonBase base class of the ButtonContent object. The former was bound in the XAML to a method on the MainWindow class in the View, code behind.

<b:ContentToggle Name="Toggle" Height="30"
                 Options="{Binding ToggleButtonView.Options,
                            RelativeSource={StaticResource View}}"
                 IsChecked="{Binding ToggleButtonView.InitialState,
                            Mode=OneWay,
                            RelativeSource={StaticResource View}}" />
<b:ContentButton x:Name="Plain" Height="30"
                 Options="{Binding PlainButtonView.Options,
                            RelativeSource={StaticResource View}}" />

The RelativeSource resource is defined in Resource Dictionary in App.xaml.

<!--Get a reference to the window to establish View Context-->
<RelativeSource x:Key="View" Mode="FindAncestor"
            AncestorType="{x:Type Window}" />

The second dependency was due to subscribing to the Click event inherited from ButtonBase in the MainWindow class.

Plain.Click += (s, e) => {
    Toggle.IsEnabled = !Toggle.IsEnabled;
};

By adding a custom DP called InitialState on ContentToggle and binding to that instead then using it to initialise IsChecked in the Loaded event, one of the dependencies was eliminated.

public static readonly DependencyProperty
    InitialStateProperty = DependencyProperty.Register(
    "InitialState", typeof(bool?), typeof(ContentToggle),
    new PropertyMetadata(false));

public bool? InitialState
{
    get { return (bool?) GetValue(InitialStateProperty); }
    set { SetValue(InitialStateProperty, value); }
}

The second dependency was eliminated by reimplementing the Click event in the custom property and routing the original event through it.

public static readonly RoutedEvent ClickEvent = EventManager.RegisterRoutedEvent(
    "Click", RoutingStrategy.Bubble, typeof(RoutedEventHandler), typeof(ContentToggle));

public new event RoutedEventHandler Click
{
    add { AddHandler(ClickEvent, value); }
    remove { RemoveHandler(ClickEvent, value); }
}

void RaiseClickEvent (object o, RoutedEventArgs e)
{
    RoutedEventArgs newEventArgs = new RoutedEventArgs(ContentToggle.ClickEvent);
    RaiseEvent(newEventArgs);
}
public ContentToggle()
{
    base.Click += RaiseClickEvent;

}

As a result, the consuming app is unaware of the primitive components with dependencies only on the CC Library.

Is this a good idea or a bad one? Is it worth the extra code?

Well, this looks like a really bad idea.

To begin with... what's the point? I mean what is so bad about having a dependency on standard .Net assembly? It is already installed on the machine as part of .Net installation, so this dependency does not affect the size of your app. Am I missing something?
This design works under fairly naive assumption, that the app will not change in any way as the time goes on. And it might be the case for your particular app, but it is not the case in general. Imagine I am using your library, and I want to subscribe to ContentToggle.Loaded event. What should I do? Should I call you at 3AM and ask you to also override this event with new keyword? Will you do the same thing for every single event (or property) found on FrameworkElement? Will you then test every single interaction, to make sure you did not break standard behavior? If you are not willing to take this all the way - don't do it at all.
I don't even want to begin imagining all the potential pitfalls. What will happen if someone sets e.Handled = true; for your new Click event? Will your base event keep going? What will happen, if I have a style for regular button that uses <EventSetter Event="Click" .../> and I want to override this setter for your button's style? How is it going to work, will I have two event handlers as a result or just one? Etc, etc.


To check if a string C is an interleaving of A and B Code This is my code to check if a string C is an interleaving of Strings A and B. Please suggests optimizations, and where I can improve.

#include <vector>
#include <list>
#include <map>
#include <set>
#include <deque>
#include <stack>
#include <bitset>
#include <algorithm>
#include <functional>
#include <iostream>
#include <iomanip>
#include <cstdio>
#include <cmath>
#include <cstdlib>
#include <ctime>
#include<string.h>

using namespace std;
int isInterleaved(char a[100],char b[100],char c[200],int i, int j, int k)
{
    if(i==0&&j==0)
        return true;
    if(i>=0&&j>=0)
    {

    if(c[k]==a[i]&&c[k]!=b[j])
        return isInterleaved(a,b,c,i-1,j,k-1);
    else if(c[k]==b[j]&&c[k]!=a[i])
        return isInterleaved(a,b,c,i,j-1,k-1);
    else if(c[k]==a[i]&&c[k]==b[j])
        return isInterleaved(a,b,c,i-1,j,k-1)||isInterleaved(a,b,c,i,j-1,k-1);
    else
        return false;
    }
    else
    {
        if(i<0&&j>=0)
        {
            if(c[k]==b[j])
            return isInterleaved(a,b,c,i,j-1,k-1);
            else
                return false;
        }
        if(j<0&&i>=0)
        {
            if(c[k]==a[i])
                return isInterleaved(a,b,c,i-1,j,k-1);
            else
                return false;
        }

    }
}
int main()
{
char a[100],b[100],c[200];
cin>>a;
cin>>b;
cin>>c;
int l1=strlen(a),l2=strlen(b),l3=strlen(c);
if(l3>l1+l2)
    {
        printf("C is not an interleaving of A and B\n");
        return 0;
    }
int ans=isInterleaved(a,b,c,l1-1,l2-1,l3-1);
printf("%d\n",ans);
return 0;
}

Just about every #include in this code is unneeded. You use cin, strlen and printf. That requires just <cstring>, <cstdio>, and <iostream>. In fact, if you're using C++, you should just use cout as an output stream instead of using printf, which would cut the imports down to <cstring> and <iostream>. The rest should be completely removed.

Why hardcode the array lengths as arguments?

int isInterleaved(char a[100],char b[100],char c[200],int i, int j, int k)

This should be:

int isInterleaved(char *a, char *b, char *c, int i, int j, int k)

Preferably, if you're using C++ and not C, you should instead use std::string (which would need an #include <string>.)

You're using a fairly strange (and inconsistent) brace style:

if(l3>l1+l2)
    {
        printf("C is not an interleaving of A and B\n");
        return 0;
    }

Or no indentation:

if(i>=0&&j>=0)
{

if(c[k]==a[i]&&c[k]!=b[j])
    return isInterleaved(a,b,c,i-1,j,k-1);
else if(c[k]==b[j]&&c[k]!=a[i])
    return isInterleaved(a,b,c,i,j-1,k-1);
else if(c[k]==a[i]&&c[k]==b[j])
    return isInterleaved(a,b,c,i-1,j,k-1)||isInterleaved(a,b,c,i,j-1,k-1);
else
    return false;
}

and

if(c[k]==b[j])
return isInterleaved(a,b,c,i,j-1,k-1);

This makes the code harder to read.

Your code could probably do with some commenting, at least explaining the different cases.

In the end, using some C++ features, I'd do it something like this:

#include <iostream>
#include <string>
#include <iterator>

template <typename Iterator>
bool is_interleaved_impl(Iterator a_begin, Iterator a_end, Iterator b_begin,
                         Iterator b_end, Iterator c_begin, Iterator c_end)
{
    // Case 1: We've exhausted a, so all we can do is draw the rest of the characters
    // from b, in order.
    if(a_begin == a_end) {
        if(std::distance(b_begin, b_end) != std::distance(c_begin, c_end)) {
            return false;
        }
        return std::string(c_begin, c_end) == std::string(b_begin, b_end);
    }
    // Case 2: We've exhausted b, so all we can do is draw the rest of the
    // characters from a, in order.
    else if(b_begin == b_end) {
        if(std::distance(a_begin, a_end) != std::distance(c_begin, c_end)) {
            return false;
        }
        return std::string(c_begin, c_end) == std::string(a_begin, a_end);
    }
    // Case 3: Characters can be chosen from either a OR b. In this case, we split into
    // 2 possibilities == 2 recurive calls.
    else if(*a_begin == *c_begin && *b_begin == *c_begin) {
        return is_interleaved_impl(a_begin + 1, a_end, b_begin, b_end, c_begin + 1, c_end) ||
               is_interleaved_impl(a_begin, a_end, b_begin + 1, b_end, c_begin + 1, c_end);
    }
    // Case 4: Only the next character from a matches.
    else if(*a_begin == *c_begin) {
        return is_interleaved_impl(++a_begin, a_end, b_begin, b_end, ++c_begin, c_end);
    }
    // Case 5: Only the next character from b matches.
    else if(*b_begin == *c_begin) {
        return is_interleaved_impl(a_begin, a_end, ++b_begin, b_end, ++c_begin, c_end);
    }
    // Nothing matches, so they aren't interleaved.
    return false;
}

bool is_interleaved(const std::string& a, const std::string& b, const std::string& c)
{
    if(a.size() + b.size() != c.size()) {
        return false;
    }
    return is_interleaved_impl(a.begin(), a.end(), b.begin(), b.end(), c.begin(), c.end());
}


Printing out a tree set using pattern matching in Scala I'm working on some exercises Scala by Example and I've implemented an excl() method on sets which removes an element from a set, and a toString method that converts a set to a string.

I'm practicing use pattern matching here. A bit concerned about shadowing my instance variables on the line

case NonEmptySet(elem, left, right)

Is there anything obvious that could be improved?

trait IntSet {
  def incl(x: Int): IntSet
  def excl(x: Int): IntSet
  def contains(x: Int): Boolean
}

case object EmptySet extends IntSet {
  def contains(x: Int): Boolean = false
  def incl(x: Int): IntSet = new NonEmptySet(x, EmptySet, EmptySet)
  def excl(x: Int) = this
  override def toString = "{}"
}

case class NonEmptySet(elem: Int, left: IntSet, right: IntSet) extends IntSet {
  def contains(x: Int): Boolean =
    if (x < elem) left contains x
      else if (x > elem) right contains x
      else true
  def incl(x: Int): IntSet =
    if (x < elem) NonEmptySet(elem, left incl x, right)
      else if (x > elem) NonEmptySet(elem, left, right incl x)
      else this
  def excl(x: Int): IntSet = {
    def combine(left: IntSet, right: IntSet): IntSet = {
      left match {
        case EmptySet => right
        case NonEmptySet(elem, ll, lr) => NonEmptySet(elem, combine(ll, lr), right)
      }
    }
    if (x < elem) new NonEmptySet(elem, left excl x, right)
    else if (x > elem) new NonEmptySet(elem, left, right excl x)
    else combine(left, right)
  }
  override def toString(): String = {
    def listElements(comma: Boolean, set: IntSet) : String = {
      set match {
        case EmptySet => ""
        case NonEmptySet(elem, left, right) => {
          val rightPart = elem + listElements(true, right)
          val list = left match {
            case EmptySet => rightPart
            case NonEmptySet(le, ll, lr) => listElements(false, left) + ", " + rightPart
          }
          (if (comma) ", " else "") + list
        }
      }
    }
    '{' + listElements(false, this) + '}'
  }

}

var subset = EmptySet incl 2
var set = subset incl 3 incl 4 incl 8 incl 9 incl 10 incl 11 excl 4
println(set contains 6)
println(set.contains(6))
println(set contains 2)
println(set incl 4 incl 6 excl 2)

Some ideas:

Make the trait sealed.
combine doesn't need to be a nested function, this only makes it harder to read and causes problems with shadowed variables. Instead you could move it to an accompanying object, or make it a separate method such as:

sealed trait IntSet {
    ...
    private def combine(withRight: IntSet): IntSet;
}

for EmptySet it will just return withRight, for a NonEmptySet it will be

 def combine(withRight: IntSet): IntSet =
    NonEmptySet(elem, left.combine(right), withRight);

toString is somewhat complicated, and can suffer from performance problems due to repeatedly concatenating strings together. Instead I'd suggest you to implement Traversable. Not only this will make your class much more useful, it's implementation will be cleaner, because it won't be cluttered by operating with strings. And then you'll already have mkString which you can use as

override def toString: String = mkString("{", ",", "}")