Changing the type of an object inside a method? First off, this is not my code.  I'm curious what everyone thinks about changing the type of an object inside of a method then modifying the properties of the cast type. I wanted to use composition to build up a patient, but this is the recommend path.  I'm probably wrong which is why I wanted 3rd party feedback.  Or maybe we just have different coding styles...

    // Person/Patient Builder
public static void BuildUpPersonOrPatient(Person person, Dictionary<string, string> row)
        {
    person.FirstName = row["FirstName"];
    person.LastName = row["LastName"];

    Patient patient = person as Patient;

    if (patient != null)
    {
        patient.PatientID = row["PatientID"];
    }
}

for reference

public class Person
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

public class Patient : Person
{
    public string PatientID { get; set; }
}

//
//

It stead of having a static method the basically does what polymorphism could do for you use polymorphism.

public class Person
{
    public string FirstName { get; private set; }
    public string LastName { get; private set; }

    public virtual void InitializeFromDictionary(Dictionary<string, string> row)
    {
        FirstName = row["FirstName"];
        LastName = row["LastName"];
    }
}

public class Patient : Person
{
    public string PatientID { get; private set; }
    public override void InitializeFromDictionary(Dictionary<string, string> row)
    {
       base.InitializeFromDictionary(row)
       PatientID = row["PatientID"];
    }
}

and then where ever you call

BuildUpPersonOrPatient(p,row);

do this instead

p.InitialieFromDictionary(row);

I've added an access modifier to the setters because unless you have a reason to be able to change those fields from outside the object you should encapsulate the possibility of chaning the state of an object. (It's surprising how seldom you have a modeling reason for that and not just a "for conviniece" reason)


Sort files by the type of file I need to sort files by the type of the File. To accomplish the task I first wrote the code below:

private static Comparator SortByFileType() {

    return Comparator.comparing((File f) -> {
        String probeContentType = null;
        try {
            probeContentType = Files.probeContentType(f.toPath());
            if (probeContentType ==null) {
                probeContentType = "probeContentTypeIsNull";
            }
        } catch (IOException e) {
        }
        return probeContentType;
    });
}

Then I needed to perform different sorts, so I decided to make an enum of sorts, but I wasn't able to pass the above code to the enum. I instead rewrote the above code as given below:

public enum Sort implements Comparator<File> {

/**
 * Compare files by the name of the File A<->Z
 *
 */
ByFileName {

        },
/**
 * Compare Files by the Type of file
 */
ByFileType {

        @Override
        public int compare(File f1, File f2) {
            String fileType1 = "";
            String fileType2 = "";

            try {
                fileType1 = Files.probeContentType(f1.toPath());
                fileType2 = Files.probeContentType(f2.toPath());
            } catch (IOException ex) {

            }

            if (fileType1 == null) {
                fileType1 = "probeContentTypeIsNull";
            }
            if (fileType2 == null) {
                fileType2 = "probeContentTypeIsNull";
            }

            return fileType1.compareTo(fileType2);
        }

    }
}

Now I need to know two things:

Can we turn the code passed to the ByFileType enum into a lambda?
Can the code passed to the ByFileType be optimized and improved?

This is a great concept to work with. Creating an enum of comparators is a clever way to make it easy to manage the sort order of file collections.

The name concerns me. I realize that Sort is convenient, but enums are normally given a "noun" name. "Sort" is a verb - something you do, not a noun which is a "thing". I would call it Sorter, or even FileSorter. Adding File to it makes code more readable.

Additionally, enum members are normally in all-uppercase and under-score separated.... so, BY_FILE_NAME instead of ByFileName.

Adding the "File" to be FileSorter you no longer need to have File as part of the enum names, so you will have: FileSorter.BY_NAME.

I believe you have truncated some code from the ByFileName enum. It does not compile without the override of the compare method. I'll just claim it is because of how you summarized things for this question.

Now, about the functional aspects. I agree, the override is a mess, and can be sorted out quite easily. First up, what you want to do is pass a comparator in as the constructor for the enum, and then use that comparator as a tool in the compare function.... like this:

private final Comparator<File> comp;

private FileSorter(Comparator<File> comp) {
    this.comp = comp;
}

@Override
public int compare(File o1, File o2) {
    return comp.compare(o1, o2);
}

That makes the actual enum guts really simple, not it's just a case of setting up the enum members.... the ones that are easy are things like size, and name sorts:

BY_NAME(Comparator.comparing(File::getName))

and

BY_SIZE(Comparator.comparingLong(File::length))

Taking your base code though, for sorting by type, you had:

return Comparator.comparing((File f) -> {
    String probeContentType = null;
    try {
        probeContentType = Files.probeContentType(f.toPath());
        if (probeContentType ==null) {
            probeContentType = "probeContentTypeIsNull";
        }
    } catch (IOException e) {
    }
    return probeContentType;
});

That code is 'ugly' because it is hard to follow the logic through, the empty catch block is a mess, and the blocked lambda (File f) -> {....} type expressions make me want to create a function for them, so I do....

I created the function:

private static final String extractType(File f) {
    try {
        String probeContentType = Files.probeContentType(f.toPath());
        return probeContentType == null ? "probeContentTypeIsNull" : probeContentType;
    } catch (IOException e) {
        return null;
    }
}

Note how I have a correctly-scoped probeContentType variable, and I use a return in both the successful, and catch-side of the try/catch block. The logic is now very visible. That function extracts just the file's type. How is it used, though?

BY_TYPE(Comparator.comparing(FileSorter::extractType)),

Putting this all together, I have the following code which I believe is quite simple, and readable:

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Comparator;

public enum FileSorter implements Comparator<File> {

    BY_TYPE(Comparator.comparing(FileSorter::extractType)),
    BY_NAME(Comparator.comparing(File::getName)),
    BY_SIZE(Comparator.comparingLong(File::length));

    private static final String extractType(File f) {
        try {
            String probeContentType = Files.probeContentType(f.toPath());
            return probeContentType == null ? "probeContentTypeIsNull" : probeContentType;
        } catch (IOException e) {
            return null;
        }
    }

    private final Comparator<File> comp;

    private FileSorter(Comparator<File> comp) {
        this.comp = comp;
    }

    @Override
    public int compare(File o1, File o2) {
        return comp.compare(o1, o2);
    }
}


Partition array in at most k subarrays that minimize a function I have 2 values \$N\$ and \$K\$ (\$1 \le K \lt N \le 1.000\$), where \$N\$ indicates a number of values (\$0 \le \text{value}2 \le ... \le \text{value}N \le 1,000,000\$) to be read from input. The first value isn't on the input and will always be 0.

All \$N\$ values need to be organized on at most \$K\$ groups where the sum of the groups size is minimized.

The size of a group is defined by the distance between the two most distant values in the group (MAX_VALUE - MIN_VALUE). If the group consists of only one value, its size is zero.

What is the minimum total value for the sum of the groups size if the values are organized optimally?

Here is an input sample:

5 2
1 2 5 6

Output sample:

3

My solution is a brute force search. This is a known problem. How can I do better than this solution?

#include <cstdio>
#include <queue>
#include <vector>
#include <limits>
#include <algorithm>

using groups = std::vector< std::vector<int> >;

int getTotalGroupsSize(groups &g){
    unsigned long k = g.size();

    int total = 0;

    for (unsigned long i = 0; i < k; ++i) {
        int max = std::numeric_limits<int>::min();
        int min = std::numeric_limits<int>::max();

        std::vector<int> &subGroup = g[i];

        for (int j = 0; j < subGroup.size(); ++j) {
            max = std::max(max, subGroup[j]);
            min = std::min(min, subGroup[j]);
        }

        total += (max - min);

    }

    return total;
}

int getOptimallSum(std::queue<int> &q, groups &g){

    if(q.empty())
        return getTotalGroupsSize(g);

    int front = q.front();
    q.pop();

    int minDiff = std::numeric_limits<int>::max();
    for (int i = 0; i < g.size(); ++i) {
        g[i].push_back(front);
        minDiff = std::min(minDiff,getOptimallSum(q, g));
        g[i].pop_back();
    }

    q.push(front);
    return minDiff;
}

int main(void){
    int n,k;

    while (scanf("%d",&n) != EOF) {
        scanf("%d",&k);

        std::queue<int> q;
        q.push(0);
        for (int i = 1; i < n; ++i) {
            int value;
            scanf("%d",&value);
            q.push(value);
        }

        if(k >= n){
            puts("0");
            continue;
        }

        groups g(k);
        printf("%d\n",getOptimallSum(q,g));

    }
    return 0;
}

Yes there is a better way.

The key observation is that since the array is sorted, the size of partition [p,q] is \$a_q - a_p\$, and the sum of sizes is almost \$a_N - a_1\$. Missing are gaps between right end of one partition and left end of the next one.

So the solution is to compute the set of sequential differences, and select \$K-1\$ largest. These are optimal partition points.

Otherwise, the code is clean and understandable.


Speed optimization for girth-counting algorithm I have the following girth-counting algorithm on a large 0-1 matrix and I need to optimize it for speed:

function CountG8()
M = 512
N = 1024
H = rand(0:1,M,N)
girth_8 = 0

for i=1:M-3
    for j=1:N-1
        if H[i,j]==1
            for j1=j+1:N
                if H[i,j1]==1
                    for i1=i+1:M
                        if H[i1,j1]==1
                            for j2=1:N
                                if H[i1,j2]==1&&j2!=j1
                                    for i2=i+1:M
                                        if H[i2,j2]==1&&i2!=i1
                                            for j3=1:N
                                                if H[i2,j3]==1&&j3!=j2
                                                    for i3=i+1:M
                                                        if H[i3,j3]==1&&i3!=i2
                                                            if H[i3,j]==1
                                                                girth_8 += 1
                                                            end
                                                        end
                                                    end
                                                end
                                            end
                                        end
                                    end
                                end
                            end
                        end
                    end
                end
            end
        end
    end
end
end

The algorithm runs without errors, but it is too slow.

The code is supposed to count every cycle of length 8 in a bipartite graph.

A bipartite graph is a graph of two groups of nodes. Left group, Check Nodes (CNs) and right group, Variable Nodes (VNs). Every CN from left group is allowed to be connected to a VN from right group. This is known as Directed Graph (DG). A DG has only cycles of even length 4,6,8,10,.. The Girth of the graph is the length of shortest cycle in it.

The algorithm walks through the graph in a BFS fashion searching for all possible nodes combination forming a cycle of the specified length (here 8). Matrix H in the code represents the graph. Each row is a CN and each column is a VN in the graph.

Here is a Bipartite Graph with its corresponding matrix as well as 2 cycles of length 4 and 6 shown:

EDIT: A comment on matrix power approach given in the answer.

In my case, the adjacency matrix is (M+N)x(M+N) = 1536 x 1536. Watch out for the complexity of (1536x1536)^8.

What you should be using is a neighboring matrix.

The key is graph theory.

You represent the graph with a matrix.

For example:

\$
\begin{bmatrix}
0 & 1 & 1 & 0 & 2 \\
1 & 0 & 2 & 0 & 0 \\
1 & 2 & 0 & 1 & 1 \\
0 & 0 & 1 & 0 & 1 \\
2 & 0 & 1 & 1 & 0 \\
\end{bmatrix}
\$

Here is a graph and its corresponding neighbouring matrix.
If you look at the first row you see that A has no one-step connection to itself. But it has a one-step connection to every other node.
Node B has a one-step connection to A and 2 different one-step connections to node C.
As you can see it all sums up if you keep going like this.

Now then, here's the mathemagic. If you multiply the matrix with itself the result will be a matrix which tells you all the connections of two-stop length.
Continue multiplying with the original matrix until you've done it 8 times and you will have a matrix which tells you how many connections of eight-step length you have.

How is matrix multiplication done, you might ask?
Wikipedia
Another site (with examples)
Basically, you multiply rows with columns.(It would take another whole answer to explain it)

When you've gotten the neighbouring matrix of the eight power, you just sum up all the elements of the diagonal. That will tell you the number of cycles of length 8. (because the diagonal holds the information of a n-step way back to itself, where n is the power of which the matrix has been multiplied)

This method works for all graphs. All you need to do is make sure the graph is bipartite to begin with.


Java beans performance with object inside object I am developing a bean class which has multi face properties (name has first name, last name and middle name). The object is going to be used to create an xml file. Please compare my two methods to incorporate the same in my application.

Method 1:

public class SampleVO {
String firstName;
String middleName;
String lastName;
// getters and setters
}

Method 2:

//Main object
public class SampleVO1 {
NameVO name = new NameVO();
// getters and setters
}
//Name object
public class NameVO {
    String firstName;
    String middleName;
    String lastName;
    // getters and setters
}

Note: Above is just a sample. The requirement is to group a set of properties. In Which way of the above performance/code standards will be good?

From a performance perspective both approaches are comparable. Yes, method 2 will use some additional memory and require a few more instructions for instantiating your NameVO, but unless you're dealing with millions of objects, I would not worry too much about that.

From a coding standards perspective, method 2 is superior over method 1. In general, composite values (such as names) deserve their own object.

First of all, it allows you to add additional methods in your NameVO, such as the likely String getFullName(), which handles the possibility of having null as a middleName. This also gives you the flexibility to change the contents of a NameVO later on, for example by adding initials, salutations, etc.

Secondly, it gives you the option to reuse NameVO in other contexts. For example, you could have a Customer object with a name field, and a Supplier with a name field, etc.

Thirdly, if your code uses validation, you can decouple the validation of the contents of a name from the validation of the existence of a name.

Furthermore, by adding an equals and hashCode, you also gain the possibility to store your objects in a Map, indexed by NameVO.

As an aside, I would even go so far as to suggest a slightly modified version of your method 2:

public class SampleVO {
    NameVO name;
    // getters and setters
}

//Name object
public class NameVO {
    final String firstName;
    final String middleName;
    final String lastName;

    public NameVO(String firstName, String middleName, String lastName) {
      // assignments
    }

    // getters
}

This effectively makes your NameVO immutable, making it more suitable for use as a key in a Map. It also makes it safer to pass NameVO objects around without having to fear side effects.


How can I shorten this paint program? I am currently in a challenge with my buddy to see who can code a simple paint program in the least lines. The only requirement, is that the program must include an eraser. How can I possibly shorten this code, while still using proper, beautiful syntax?

import sys, pygame
from pygame.locals import *
pygame.init() #Starts pygame
screen = pygame.display.set_mode((1000,720)) #window, and sets the size
screen.fill((255,255,255)) # Fills background color
brush = pygame.image.load("brush.jpg") #Loads the image into a variable
eraser = pygame.image.load("white.png") #Loads the image into a variable
brush = pygame.transform.scale(brush, (10,10)) #Scales the image into a more useable
eraser = pygame.transform.scale(eraser, (100,100)) #Scales the image into a more
clock = pygame.time.Clock() #Makes a clock to track the ticks within the game
z = 0
while True:
    clock.tick(60) #Limits the ticks to 60 (FPS)
    x,y = pygame.mouse.get_pos() #Sets two variables for the mouse position
    for event in pygame.event.get(): #Recieves events
        if event.type == QUIT: #Checks if the event is a QUIT event
            pygame.quit()  ##Quits##
            sys.exit()     ##Quits##
        elif event.type == MOUSEBUTTONDOWN:
            z = 1 #If you press the mouse button down, it sets the screen blit to true
        elif event.type == MOUSEBUTTONUP:
            z = z - 1 #Does the opposite of the above elif statement
        if z == 1: #Cheks if the variable z is true, if it is; updates the screen with the brush
            screen.blit(brush,(x-5,y-5))
        if event.type == KEYDOWN:
            screen.blit(eraser,(x-5,y-5))

    pygame.display.update()

Here's my best effort (8 lines):

from pygame import *
init()
screen = display.set_mode((1000,720))
for e in iter(event.wait, event.Event(QUIT)):
    col = {(1, 0, 0): 'white', (0, 0, 1): 'black'}.get(mouse.get_pressed())
    if col and e.type in (MOUSEBUTTONDOWN, MOUSEMOTION):
        display.update(screen.fill(Color(col), Rect(mouse.get_pos(), (20, 20))))
quit()

Notes:

from module import * is normally deprecated because you don't know all the names you're importing, and some of these might conflict with or shadow names from other modules. But in simple cases like this, with no other dependencies, it's justified because it results in code that's easy to read. You can always go through and change X to pygame.X if you want.
In this program there's nothing that animates, so there's no need for a clock, and the display only needs to be updated when it changes.
You didn't post your images brush.jpg and white.png so I've used solid colour rectangles. (Also, why a JPEG? JPEGs are lossy, so not appropriate for bitmap graphics.)
By changing the program to draw white-on-black I can avoid the initial screen.fill and save a line.
I've used colour names 'white' and 'black' which are clearer than RGB tuples like (255, 255, 255).
I've used the two-argument form of iter to avoid the test for event.type == QUIT. (This feature is useful when you want to exit an iteration on a special value.)
By using event.wait instead of event.get I can reduce the two loops to one.
I've changed the user interface so that the left mouse button draws and the right mouse button erases. This is done by looking up the tuple returned by mouse.get_pressed in a dictionary, to get the colour for the fill.
Surface.fill returns the rectangle that was filled, which is exactly the part of the display that needs to be updated, so I can pass it directly to display.update.


Sound Control System This is my sound system that allows for the following functionality:

One master volume that controls every sound.
A function to stop/pause every playing sound
A function to control every instance of a sounds volume(@creation)
A function to stop/pause every instance of the same sound
Sounds that are playing are kept in a list to keep track of.
retrieve a list of specified sounds playing
Retrieve a list of every sound playing

How could I possibly improve the code, or the system itself? I could possibly allow the user to define layers to encapsulate sectors of sound and not just the same instance. I could probably make a user.

  // exponentially decrease sounds
  this.Sound.masterVolume = function(v){ masterVolume = Math.pow(v,2);};
  this.Sound.instanceVolume = function(name, v){ instanceVolume[name] = Math.pow(v,2);};
  this.Sound.masterStop = function(){
    liveSounds.forEach(function(snd){
      snd.pause();
      snd.onended();
      snd.currentTime = 0;
    });
  }
  this.Sound.masterPause = function(){
    liveSounds.forEach(function(snd){
      snd.pause();
    });
  };
  this.Sound.instanceStop = function( name ){
    var match = sounds[name].src;
    liveSounds.forEach(function(snd){
      if( snd.src !== match ) return;
      snd.pause();
      snd.onended();
      snd.currentTime = 0;
    });
  };
  this.Sound.instancePause = function( name ){
    var match = sounds[name].src;
    liveSounds.forEach(function(snd){
      if( snd.src !==  match ) return;
      snd.pause();
    });
  }
  this.Sound.instanceGetLive = function( name ){
    return liveSounds.filter(function(snd){
      return( snd.src === match );
    });
  }
  this.Sound.masterGetLive = function(){
    return liveSounds.splice();//only a copy.
  }

  this.sound = function(name){
    var snd = new Audio( sounds[name].src ),
        that = this;
    snd.volume = instanceVolume[name] || 1;
    snd.play = function(){
      this.volume *= masterVolume;
      Audio.prototype.play.apply(this);
      liveSounds.push(this);
      this.onended = function(){ liveSounds.splice( liveSounds.indexOf(snd), 1 );};
    };
    return snd;
  };

You can get really far cleaning this up by just creating some private functions and bumping them to the bottom of the file so that they can be hoisted

  this.Sound.masterVolume = function(v){ masterVolume = Math.pow(v,2);};
  this.Sound.instanceVolume = function(name, v){ instanceVolume[name] = Math.pow(v,2);};
  this.Sound.masterStop = function(){ liveSounds.forEach(reset) }
  this.Sound.masterPause = function(){ liveSounds.forEach(pause) }
  this.Sound.instanceStop = function( name ){ liveSoundsExcept(name).forEach(reset) }
  this.Sound.instancePause = function( name ){ liveSoundsExcept(name).forEach(pause) }
  this.Sound.instanceGetLive = function( name ){
    //this seems to be an error - what is match?
    return liveSounds.filter(function(snd){
      return( snd.src === match );
    });
  }
  this.Sound.masterGetLive = function(){
    return liveSounds.slice(0);//slice generates a copy - I don't think splice does
  }

  this.sound = function(name){
    var snd = new Audio(getSound(name)); //reference to `this` removed because it wasn't being used
    snd.volume = instanceVolume[name] || 1;
    snd.play = function(){
      //why use the ambiguous `this` here? It can vary depending on how this is called - you already have an isntance of `snd`!
      snd.volume *= masterVolume;
      //Audio.prototype.play.apply(snd); //Why? you can just use
      snd.play();
      liveSounds.push(snd);
      // This can't be right - you don't need to do any cleanup?
      // this.onended = function(){ liveSounds.splice( liveSounds.indexOf(snd), 1 );};
    };
    return snd;
  };

  function getSound(name) { return sounds[name].src}
  function liveSoundsExcept(name) {
    var match = getSound(name);
    return liveSounds.filter(function(snd){ return snd.src !== match });
  }
  function pause(snd) { snd.pause(); }
  function reset(snd) {
    pause(snd);
    snd.onended();
    snd.currentTime = 0;
  }

I absolutely agree with @Schism that more context is needed for a really good review but I made a few other observations inline.

One that also jumps out at me is this whole relationship between liveSounds and how you're selecting by name. Without more context I can't say for sure, but it seems like maybe you can have liveSounds = {} with the name being the key and that would make everything much simpler.

You do need to think if sounds need to get disposed of or what happens if you invoke a method for something without a name, or if there's two things with the same name.

Also, as always, I'm going to go ahead and say the prodigious use of this here is probably entirely unnecessary and you can just use a factory method instead of a constructor.

Also, it's 2015, can't we just start using es6 already?


Using a bool flag to determine whether in root-level or not I have several situations in which I need to write a recursive function that does things. I'm asking about one special pattern that I use in recursive methods from time to time.
But first an example, so that you know what I'm talking about.

Example

Assume the following C# code:

class Item
{
    public int Id { get; set; }
    public int ParentId { get; set; }
    public string Name { get; set; }
}

void Main()
{
    var items = new List<Item>
    {
        new Item{Id = 1, ParentId = 0, Name = "Root"},
        new Item{Id = 2, ParentId = 1, Name = "Sub1"},
        new Item{Id = 3, ParentId = 2, Name = "Sub2"},
        new Item{Id = 4, ParentId = 2, Name = "Sub3"},
        new Item{Id = 5, ParentId = 2, Name = "Sub4"},
        new Item{Id = 6, ParentId = 4, Name = "Sub5"}
    };

    Console.WriteLine(GetPath(items, items[5]));
}

string GetPath(List<Item> list, Item current, bool inRecursion = false)
{
    if (current == null)
    {
        return "";
    }

    var res = GetPath(list, list.FirstOrDefault(m => m.Id == current.ParentId), true) + "/" + current.Name;

    if (!inRecursion)
    {
        res = res.TrimStart('/');
    }

    return res;
}

Explanation

The GetPath method tries to construct a path from one item down to a root item.
It gets a list of all items (list), where each item has an Id, a ParentId and a Name.  As second parameter, it takes an item to start with (current).
When called, it calls itself, but with the parent item as current item.
Finally, when an item has no parent, it returns an empty string. The return values from the recursive calls are then appended to the current item's name.

(It's always a little bit hard to explain recursion, I hope it's understandable...)

So, the code above gives me Root/Sub1/Sub3/Sub5 as result.

Question

As you see, the method has a third parameter (inRecursion), which indicates whether the method is in recursion, or not. I use it, to remove the leading / in the path.

I see myself using a boolean flag like this in recursive methods from time to time, but I wonder if this is the way to go, or if there is a better way to solve the problem of indicating the first call, where the method hasn't called itself. Using a flag like this seems somehow dirty to me.

So, is this considered bad practice, or am I all fine with using a flag like this?

As you see, the method has a third parameter (inRecursion), which indicates whether the method is in recursion, or not. I use it, to remove the leading / in the path.

You need this flag because your function is doing to much. It violates the S (Single responsibility principle) in SOLID. It's not only resolving the elements but it's building the final result at the same time. The method should actually only return a List<string> with all the names (or other itmes) it found so that you can do what ever you need to do with it like:

var names = GetNames(...);
var path = string.Join("/", names);

It doesn't matter if you use recursion or a stack or any other method. The most important part is that it should only do one thing: return a collection of items.

What if you wanted to use a different delimiter such as \ or .? You'd need a new parameter for it. However, if you only have a list of names it's much easier to change it and build a different path because this is another responsibility.


Expressing positive integers as the sum of different charming numbers In a practice academic interview of mine, we discussed question six, round one, of the United Kingdom Mathematics Trust's 2015 British Mathematical Olympiad. Which states:

  A positive integer is called charming if it is equal to 2 or is of the form
  3i5j where i and j are non-negative integers. Prove that every positive integer can be written as a sum of different charming integers.

Having been able to successfully prove this, afterwards I then went on to implement, in Python, a program which can express any positive integer in terms of the sum of different charming numbers.

To do this I start by converting the integer into base 3 so that it is easier to find a charming number that is more than half the value of the original integer, but less than it. This value is then appended to a list, and the process is then repeated with the difference until left with either 1, 2, or 3. A list of charming numbers is then returned, which all sum to the original number.

I know this method works, simply as I used it somewhat in my proof.

I apologise in advance for the lack of comments.

def to_base_3(base_10: int) -> str:
    working_number = int(base_10)
    output = ''

    while True:
        next_digit = working_number % 3
        output = str(next_digit) + output
        working_number = working_number // 3

        if working_number == 0:
            return output

def to_base_10(base_3: str) -> int:
    output = 0

    for i, char in enumerate(base_3[::-1]):
        output += int(char) * (3 ** i)

    return output

def find_charming_components(number: int, charming_components: list = None) -> list:
    if charming_components is None:
        charming_components = []

    base_3_value = to_base_3(number)
    digit = base_3_value[0]
    component = 0

    if len(base_3_value) == 1:
        if digit != '0':
            charming_components.append(int(digit))

        return charming_components

    if digit == '1':
        component = to_base_10('1' + '0' * (len(base_3_value) - 1))
        # Find the largest power of three that is lower than the current value. I.e: 3**4
        charming_components.append(component)
        # Append this charming number to the list of components

    elif digit == '2':
        component = to_base_10('12' + '0' * (len(base_3_value) - 2))
        # Find the largest power of three times five that is lower than the current value. I.e: 3**4 * 5
        charming_components.append(component)
        # Append this charming number to the list of components

    number -= component
    # Repeat process with the difference
    return find_charming_components(number, charming_components)

print(find_charming_components(int(input('Number: '))))

I just feel like doing a full base 3 conversion and back again isn't the most efficient method of doing this, and would appreciate some help on generally improving the algorithm.

Your to_base_10(x) function may be easily rewritten as:

def to_base_10(base_3):
    return int(base_3, 3)

However, you are only using the function to convert base 3 numbers of the forms '1' followed by n zeros, and '12' followed by n zeros.  These can be directly computed as:

to_base_10('1' + '0'*n) -->  3 ** n
to_base_10('12' + '0'*n) --> 5 * 3**n

The output of the to_base_3(x) function is only used to produce 2 values: len(base_3_value) and digit = base_3_value[0].  These can also be directly computed.

if number > 0:
    len_base_3_value = int(math.log(number, 3)) + 1
    digit = number // (3 ** (len_base_3_value - 1))
else:
    len_base_3_value = 1
    digit = 0

Note: digit is now an int (0, 1, or 2), not a str ('0', '1', or '2')

You recursively call and then return the value of find_charming_components(number, charming_components).  Python does not do tail recursion optimization, so this should be replaced with a simple loop, instead of recursion.


Checking if a string contains all unique characters I am currently reviewing CTCI(Cracking the Coding Interview)
The question is asking me to implement an algorithm which checks whether the characters in a string are all unique however they do not want me to use any auxillary data structures
My Algorithm is relatively straight forward, Two nested loops one starting at 0(i)  and another at 1(i+1)
If the condition finds two characters that are equal then I print duplicate characters have been found.
 public static void checkUnique(String s)
{

    for(int i = 0; i<s.length(); ++i)
    {
        for(int j = i +1; j<s.length(); ++j)
        {
            if(s.charAt(i) == s.charAt(j))
            {
                System.out.println("Duplicates found");
            }
        }
    }

}

My question

Is this an optimal algorithm, my second approach was to sort, and find a  pair which contains the same characters.
Obviously a hashmap would be beneficial here, but questions does not really want me to use it.

I have refactored the code and hopefully now it is correct.
Is there any way of reducing it to an O(N) runtime. What if we keep track of the Unicode code's for each char character. I doubt O(N^2) is the best we can do here.

A way to optimize it would be to return as soon as you have found a duplicate, unless the question specifically asks you to list all duplicates.
You could refactor it to something like that:
public static boolean checkUnique(String s) {
    int len = s.length();
    for (int i = 0; i < len; i++) {
        int c = s.codePointAt(i);
        int next = s.indexOf(c, i + 1);
        if (next > i) {
             return false;
        }
    }
    return true;
}

It isn't more efficient as your solution, but I doubt there is an algorithm that can do it better.


Database connection in MVC model I followed the tutorial Build a PHP MVC Application and have implemented a database connection. I made a seperate database connection class and I call it in the controller.

It works, but now I have to inject it whenever I call a method from the user model. Is this considered good (dependency injection) or bad (repeating same variable over and over again seems useless)?

I was thinking about moving the db_con method to the user  model itself, but then I'd have to include it in every other future model as well, which is repeating code. Perhaps I should inject it in the first controller method:return new $model(); would become return new $model($this->db_con());? I don't know anymore. So many possibilities, but where would it fit best?

    <?php
      class Controller
      {
        private $db_con = false;

        public function model($model)
        {
          require_once '../app/models/' .$model .'.php';
          return new $model();
        }

        public function view($view, $data)
        {
          require_once '../app/views/' .$view .'.php';
        }

        public function db_con()
        {
          if($this->db_con == false)
          {
            $database = new Database();
            $this->db_con = $database->db_con;
          }

          return $this->db_con;
        }

Now, in my register controller I connect to the user model and inject the database connection like this:

<?php
  class Register extends Controller
  {
    protected $user;

    public function __construct()
    {
      $this->user = $this->model('User');
      $this->db_con = $this->db_con();
    }

    public function form()
    {
      $this->view('register/form', ['notice' => ""]);
    }

    public function submit()
    {
      //Lots of if/elses, then I connect:
      elseif($this->user->create_user($this->db_con, $_POST['username'], $_POST['password'], $_POST['email'], $_SERVER['REMOTE_ADDR']))
      {
        $this->view('register/submit', ['notice' => "You have been registered."]);
      }

And to be complete, the database connection class itself:

<?php
  class Database
  {
    public $db_con = false;
    public function __construct()
    {
      try
      {
        $this->db_con = new PDO('mysql:host='. DB_HOST .';dbname=' . DB_NAME .';charset=utf8', DB_USER, DB_PASS);
      }
      catch (PDOException $e)
      {
        error_log($e->getMessage(), 0);
      }
    }
  }
?>

So here are some thoughts:

Your Database class seems to hold zero value.  It provides no abstraction away from underlying PDO if that is the desired goal (which I don't see why it should be since PDO is itself an abstraction). It literally does nothing except catch and log exceptions to instantiating a PDO object. Since it doesn't abstract away things like executing queries, prepared statements, transactions, etc. any class using this class would still need to understand that it is working with an underlying PDO object, so why hide that away inside a Database object class that does absolutely nothing?  It would be better, in my opinion. to have underlying classes understand clearly that they are working with PDO object.
You are not doing dependency injection with what you currently have. If you are trying to move toward dependency injection (which is oftentimes a good strategy), then you need to actually inject the dependency to the consuming class/code, not instantiate the dependency within the class.

So for your Controller class, that might look something like this:

class Controller
  {
    // note I use null here, which is typically better than false
    // to indicate that something is missing any value whatsoever
    protected $pdo = null;

    // note that I have specified that a valid PDO object must be passed
    // to constructor.  Without this, the object would be useless.
    public function __construct(PDO $pdo) {
        $this->pdo = $pdo;
    }

    /*
    Eliminate this method as again, you should probably pass this
    dependency to the constructor.
    Definitely don't have `include` or `require` inside class methods.
    This particular method also seems to have nothing to do with the base
    controller at all. Do you need a separate model factory?
    public function model($model)
    {
      require_once '../app/models/' .$model .'.php';
      return new $model();
    }
    */

    /*
    Eliminate this function as the class should not care about the view.
    Perhaps you have another class which take a Model object as
    dependency and can render the view
    public function view($view, $data)
    {
      require_once '../app/views/' .$view .'.php';
    }
    */

    /*
    Eliminate this method altogether as we are passing PDO object
    to the constructor.  And we don't the controller class to need
    to return its database connection to any caller.  Any other class
    that needs a DB connectoin should have that DB connectoin object
    injected into it.
    public function db_con()
    {
      if($this->db_con == false)
      {
        $database = new Database();
        $this->db_con = $database->db_con;
      }

      return $this->db_con;
    }
    */

Since your code is incomplete for this class, it is hard to say if there is something else this class should be doing.  To me at this point it is not really clear what this controller really does other than hold a DB connection. Perhaps you might consider implementing abstract methods here if the intent is to have inheriting classes provide implementation, though I see no signs of this from your code.

With regards to your Register class, you would need to change the constructor to first call the parent class constructor (to load the PDO object to $this->pdo) then load user model.  Also you have significant problem with working directly with user input ($_POST and $_SERVER variables) without any sort of validation.  My guess is that the data cleaning validation, should happen outside this class so the class need not worry about potentially working with non-validated user input.
Finally, I would like to point out that typically in MVC pattern, the controller can only manipulate the model, and it is the job of the view to take the model and render it.  So I am not sure why your controller has any relation to the view at all. The controller should typically respond to user input and manipulate the model.  Certain MVC implementations have the controller then pass the model to the view for rendering, but it seems here you are actually rendering the view with the controller, which seems a little odd (especially how you are doing it via file inclusion which seems very fragile).


Tinder-esque location pulse animation effect As shameful as it is to say I wanted to see what all the fuss about was with Tinder, but after downloading it I found I was more interested with its animation effects and started wondering how they did them!

I decided I wanted to see if I could replicate some of them using jQuery so they can be used on web apps and sites and such, but it's always nice to have these snippets lying around if I ever need them.

This seems lightweight enough, and I'm pretty sure I've covered everything, each circle element is targeted individually, the elements are removed after to stop a build up etc. But just out of curiosity, would anyone have approached it differently?

$(document).ready(function () {
    var x = 0;

    addCircle(x);

    setInterval(function () {
        if (x === 0) {
            x = 1;
        }

        addCircle(x);

        x++;
    }, 1200);
});

function addCircle(id) {
    $('body').append('

');

    $('#' + id).animate({
        'width': '300px',
        'height': '300px',
        'margin-top': '-150px',
        'margin-left': '-150px',
        'opacity': '0'
    }, 4000, 'easeOutCirc');

    setInterval(function () {
        $('#' + id).remove();
    }, 4000);
}

Example

Disclaimer: I don't know what Tinder is yet.
Shorter version:

$(document).ready(function () {
    function addCircle() {
        var $circle = $('

');
        $circle.animate({
            'width': '300px',
            'height': '300px',
            'margin-top': '-150px',
            'margin-left': '-150px',
            'opacity': '0'
        }, 4000, 'easeOutCirc');
        $('body').append($circle);

        setTimeout(function __remove() {
            $circle.remove();
        }, 4000);
    }
    addCircle();
    setInterval(addCircle, 1200);
});

I put the addCircle method inside the closure, keeping the
 in a variable means there is no need for an ID or counter x and used setTimeout because it only needs to run once.

http://jsfiddle.net/Y3r36/9/

I like it! Might have made the sizes variable but that could be overkill in this situation. Also it might be a good idea to put a wrapping
 with position:relative in the case you want multiple of these.


Check robustness of function to add an interval to a date Background of the problem

My Class level variables:

    static int secondsInterval = 100000000;
    static long secondsSpecial=0;
    static Date dateSecondsReg;
    static Integer intValue;

I have a function which uses the standard Java Calendar:

The function accepts a value in long(Which is seconds elapsed from a certain date till now)  and a date(Start date) then based on this calculates seconds elapsed for sometime in future. The problem I was running into earlier was that the
cal.add(Calendar.HOUR,intValue); would only accept int values as args and I used to add seconds earlier, now I changed it to Hours so that I dont get an Integer Overflow.

public static void secondsToNotify(long seconds, String d){
        Date dt = Convert(d);
        System.out.println(""+dt);
        Calendar cal = Calendar.getInstance();
        cal.setTime(dt);
        secondsSpecial = secondsInterval*(1+(seconds/secondsInterval));
        System.out.println(""+(secondsSpecial));

        long hour = (secondsSpecial /60)/60;
        System.out.println(""+hour);

        if(hour >= Integer.MAX_VALUE){

        }else{
            intValue =  (int) (long)hour;
        }

        cal.add(Calendar.HOUR,intValue);
        dateSecondsReg = cal.getTime();
        System.out.println(""+dateSecondsReg);
        System.out.println(""+secondsSpecial);
    }

The code in:

secondsSpecial = secondsInterval*(1+(seconds/secondsInterval));

is important for me to get an integer division to drop off the residue.

Is this fine or there is something more behind the scenes?

static long secondsSpecial=0;

Static variables that get modified? Rethink your approach.

If you have static variables, there's a good chance that either your design is wrong, or it should be an instance class. Always limit the scope of variables, methods and classes to the smallest one possible. A variable that is not used outside of the class should not be public, a function that only concerns the class it is defined in should not be public, a variable that is only used inside one function should not be declared in the class but in the function itself and so on.

public static void secondsToNotify(long seconds, String d){

What does d do? What is it for? Rename it according to what it role is, or provide thorough documentation on why it only is one letter.

System.out.println(""+dt);

Are you coming from a VB6 background? I wish I could say that the last time I saw someone using the "empty-string-concatenate-cast" was somewhere in the nineties...unfortunately I did not code back then and I work with a VB6 coder...so...

Why is that bad? It's some sort of, well, implicit type casting in the hope that it might work. That the Java compiler allows such thing is still a mystery to me.

Easier readable alternatives:

System.out.println(dt.toString()); // Turns out this does the same in a better way.
System.out.println(dt); // Calls the objects toString() method.

secondsSpecial = secondsInterval*(1+(seconds/secondsInterval));

By now I know that those variables do not have the ideal names they could have.

System.out.println(""+(secondsSpecial));

Same here...are you aware that System.out has an overload of println that accepts long?

System.out.println(secondsSpecial);

long hour = (secondsSpecial /60)/60;

I here for inform you that this static method is not threadsafe...well, that felt funny.

If this static helper method is called from two different threads, it's possible that the first thread will set secondsSpecial, print something and while the first thread prints something the second will come along and also set secondsSpecial.

...
Thread1: Set secondsSpecial to 3
Thread2: Set the time of the calendar
Thread1: Print the value of secondsSpecial
Thread2: Set secondsSpecial to 25634
Thread1: Use secondsSpecial

And that's why static variables that get modified are bad. No one would expect a static helper method to be not threadsafe if not explicitly told about. Or would you think that Arrays.asList() is not threadsafe?

if(hour >= Integer.MAX_VALUE){

}else{

Silently failing is not acceptable, this would be a good moment to throw a IllegalArgumentException with a descriptive message.

intValue =  (int) (long)hour;

hour is already a long.

System.out.println(""+dateSecondsReg);
System.out.println(""+secondsSpecial);

While we're at it, your function should not directly print to stdout. If you want to log something within a function, a Logger would be appropriate.

secondsSpecial = secondsInterval*(1+(seconds/secondsInterval));

  is important for me to get an integer division to drop off the residue.

Please correct me, but isn't that what seconds / 60 does?

Java automatically performs an integer division if it is handed two integers/long. Anyway, this solution would be hacky at best, (int)Math.floor(value) would be better.

All in all, it sounds more like you want to rewrite the function completely and instead only accept a calendar instance which you then add the long seconds to. Like this (pseudo code):

function addSecondsToCalendar(Calendar cal, long seconds) {
    long hours = seconds / 60
    if hours >= Integer.MAX_VALUE throw IllegalArgumentException

    cal.add(hours)
}

Which raises the question what you're doing that you need this functionality. Short comparison:

Integer.MAX_VALUE:  2147483647
In minutes:         35791394.116667
In hours:           596523.23527778
In days:            24855.134803241
In years:           68.096259734906

So if you only use int seconds, you can already schedule notifications (that's what your function does, right?) for the next ~68 years.

If, on the other hand, you use "historic" dates as starting point, then it is understandable that you need a bigger range than ~68 years. Should be mentioned in the JavaDoc of the method, though.


Provide binding event mechanism in revealing module pattern Goal: provide binding event mechanism which can use default or custom settings in a most proper, organised and readable way.

var module = (function () {
    var settingsDefault = {
        clickObject: 'body'
    };
    var s = settingsDefault;

    var init = function (settingsCustom) {
        s = settingsCustom;
        console.log(s);
    };

    var _click = function(clickObject){
        console.log('click occured on: ' + clickObject);
    };

    var bindEvent = (function () {

        var click = function () {
            $(s.clickObject).unbind('click').click(function() {
                _click(s.clickObject);
            });

            return this;
        };

        return {
            click: click
        }
    })();

    return {
        init: init,
        bindEvent: bindEvent
    }
})();

My concerns

Did I use a good naming convention? Particularly: If I added the underscore for private _click method should I use the underscore for all private methods then?
Is this a proper way to handle default and custom settings in revealing module pattern?
Is it a good idea to keep all binding events in a separate method of my main module, or should I rather move them one level up or move them to a new module?
If I use init method should I resign from IIFE and use module.init() method to initialize the module, or can I leave it as it is?
Other suggestions unrelated to above concerns would be also appreciated.

The names aren't too bad. Yes, if you're using underscore to indicate a private variable, then all privates should start with an underscore. I'd suggest:

_click could be _onClick
s could be settings.

You're overwriting the settings completely. If there were a few more settings, I should be able to specify the ones I care about, but leave the default ones as is.

For example:

var settingsDefault = {
    clickObject: 'body',
    onClick: function(elSelector){
        console.log('clicked occurred on ' + elSelector);
    }
};
var s = settingsDefault;

var init = function (settingsCustom) {
    s = settingsCustom;
};

var _click = s.onClick;

In the above, if I do this:

module.init({
    clickObject: '.foo'
});

it'll blow up because s.onClick is not defined. You're better off to $.extend default settings with config settings.

var init = function(settingsCustom){
    s = $.extend({}, settingsDefault, settingsCustom);
};

This isn't quite true with your code, as the bind is an IIFE, but it does demonstrate the weakness in overwriting settings, rather than extending.

I would suggest keeping all bindings happening within the module, where it is at the minute - this way, everything is in one place.
I'm not a huge fan of IIFE's - the consumer should be in control of what happens when. I'd do away with the IIFE completely, but if it suits your needs, and you're happy to have your module as a singleton, then it's absolutely fine.
Nothing else to add really. Consume your own code, see if it meets your needs - I've found that I rarely need a singleton, so it might not work as you're hoping. That said, I do use them sometimes - but not as often as I originally expected!


Pirate Bay name and magnet mailer So, first time I try PHP. Thought I'd set a goal for myself, divide it up in smaller problems and start googling. So yes I got it to work, very happy.

But, it is rather crude and ugly. So now I would like to know how it should look.
Proper regex, proper formatting in the array, keeping the links as links and any other tips.

Just to be clear, what it does: You place this on a server you have, browse to the url where the file is, it scrapes Pirate Bay, and emails you the names and magnets. It could be used by one person on his web domain as private 'proxy'. No one else can see or use it and it is all in one simple file.

I miss-posted earlier but they did say using cache would be smart.

$data = file_get_contents('http://thepiratebay.se/top/201');
$selected = '/Details.for.(.+?)"|magnet:.+?"/';
preg_match_all($selected,$data,$matches, PREG_OFFSET_CAPTURE);

$to = "me@domain.com";
$subject = "Pirate Mail";

if (mail($to, $subject, print_r($matches, 1), 'From: BlackBeard@piratemail.com')) {
   echo("
Message successfully sent!
");
  } else {
   echo("
Message delivery failed...
");
  }

Generally, RegExps are a bad choice to deal with HTML code. You're better of with DOM parser: parse the tree and then use XPath to find desired elements, i.e. the ones containing the torrent name and the magnet link. I'm not posting any code because I've never done it in PHP though, but as a pointer...

Anyway, if you just wanted a simple script that just works, you're good, and if you wanted to do some self-education, you can try parsing the DOM.

The first search result from Google gives me this: http://simplehtmldom.sourceforge.net/

And some points on the code style:

You have some weird (IMO) indentation in the if statement. Usually the closing bracket is on the same level as if is, i.e.:

if (mail($to, $subject, print_r($matches, 1), 'From: BlackBeard@piratemail.com')) {
    echo("
Message successfully sent!
");
} else {
    echo("
Message delivery failed...
");
}

And you have inconsistent spaces-after-arguments:

preg_match_all($selected,$data,$matches, PREG_OFFSET_CAPTURE);
....
mail($to, $subject, print_r($matches, 1), 'From: BlackBeard@piratemail.com')

Second one (with spaces after commas) is usually a preferred one.


Determine if a std::string has all of the same character I'm working on an algorithm to determine if a std::string has all of the same character. Here is what I have:

bool string_has_all_of_the_same_chars(const std::string& s) {
    return std::all_of(s.begin(), s.end(), [&s](char ch) { return ch == s[0]; });
}

Are there any improvements that can be made to this? I am guaranteed by another function that s is not empty.

You could simply use find_first_not_of

bool string_has_all_of_the_same_chars(const std::string& s) {
    return s.find_first_not_of(s[0]) == std::string::npos;
}

Update:

This works only, when the string is not empty. A complete version would also test empty

bool string_has_all_of_the_same_chars(const std::string& s) {
    return s.empty() || s.find_first_not_of(s[0]) == std::string::npos;
}

Instead of s[0], one could also use s.front() or *s.begin().


URL selection function that is designed to randomly assign users to different surveys until full I'm trying to condense some of my code to make it more maintainable and to learn a few new tricks.

Below is a Python 2.7 / Django 1.6 function which is designed to assign users to one of three surveys in my application. I require a total of 30 responses in total, 10 each. Each user is to be assigned at random to one of the three surveys to complete, once a survey e.g. SurveyTwo has had 10 participants it is taken out of the selection pool.

The function returns a segment of the URL which is then used to update a button

START

My below code works fine, but in reality I have nine such paths and I am looking to improve on my implementation method.

Also, please suggest any tips on styling or techniques.

PathMax = 10
TotalMax = 30
TotalMaxCounter = 0
SurveyWizardOneCounter = 0
SurveyWizardTwoCounter = 0
SurveyWizardThreeCounter = 0
SurveyWizardFourCounter = 0

SURVEY_URLS = ['/surveyone/', '/surveytwo/', '/surveythree/']

def start(request):
    global TotalMaxCounter
    if TotalMaxCounter < TotalMax:

        survey_url = random.choice(SURVEY_URLS)

        if survey_url == '/surveyone/':
            print 'SurveyWizardOneCounter is: %s TotalMaxCounter is: %s' % (SurveyWizardOneCounter, TotalMaxCounter)
            if SurveyWizardOneCounter > PathMax:
                SURVEY_URLS.pop(0)

        elif survey_url == '/surveytwo/':
            print 'SurveyWizardTwoCounter is: %s TotalMaxCounter is: %s' % (SurveyWizardTwoCounter, TotalMaxCounter)
            if SurveyWizardTwoCounter > PathMax:
                SURVEY_URLS.pop(1)

        elif survey_url == '/surveythree/':
            print 'SurveyWizardThreeCounter is: %s TotalMaxCounter is: %s' % (SurveyWizardThreeCounter, TotalMaxCounter)
            if SurveyWizardThreeCounter > PathMax:
                SURVEY_URLS.pop(2)

        return render(request, 'start.html', {'survey_url': survey_url})
    else:
        return render(request, 'surveyfull.html')

You seem to have a fair bit of hard-coding there. I would suggest instead something like:

PATH_MAX = 10
WIZARD_COUNTER = {'/surveyone/': 0,
                  '/surveytwo/': 0,
                  '/surveythree/': 0,
                  ...}

def start(request):
    if WIZARD_COUNTER:
        survey_url = random.choice(WIZARD_COUNTER)
        WIZARD_COUNTER[survey_url] += 1
        if WIZARD_COUNTER[survey_url] == PATH_MAX:
            del WIZARD_COUNTER[survey_url]
        return render(request, 'start.html', {'survey_url': survey_url})
    else:
        return render(request, 'surveyfull.html')

This makes it much easier to alter the number of surveys in the future, you just need a dictionary mapping {url: count}. If you want to have different counts, the simplest way is to set the PATH_MAX to whichever you want the most of, then preset individual counts to PATH_MAX - count_for_that_survey.


iOS: prettify random color for debugging iOS: prettify random color for debugging

I used to use random color to debug, from colors I like

extension CALayer{
      func debug(){
             let colors = [UIColor.blue, UIColor.magenta, UIColor.red,
                      UIColor.green, UIColor.brown, UIColor.purple,
                      UIColor.cyan]
             if let color = colors.randomElement().cgColor{
                     borderColor = color
              }
       }
 }

The effect case:

The colors are not of regularity. Not very pretty.

I want to prettify it. Do some color alignment. The same color for the same UI hierarchy.

achieve something like this.

In order to not make colors repeat, and to show more colors in a page. I use ring buffer.

In order to do some color alignment, the same color for the same UI hierarchy，I use call stack trace. Use the call function as the key for its color.

Here is the code:

// the color ring buffer
struct ColorCluster {
    static var shared = ColorCluster()

    let colors = [UIColor.blue, UIColor.magenta, UIColor.red,
                  UIColor.green, UIColor.brown, UIColor.purple,
                  UIColor.cyan]

    var colorRegister = [String: Int]()
    var count = -1

    mutating func get(_ k: String) -> CGColor{
        var index = 0
        if let idx = colorRegister[k]{
            index = idx
        }
        else{
            count += 1
            if count >= colors.count{
                count = 0
            }
            index = count
            colorRegister[k] = count
        }
        return colors[index].cgColor
    }
}

extension CALayer{

    func debug(){
        // call stack trace, remove methods of the system
        let libs: Set<String> = ["UIKitCore", "QuartzCore", "CoreFoundation"]
        var methods = [String]()
        Out: for symbol in Thread.callStackSymbols{
            for lib in libs {
                if symbol.contains(lib){
                    break Out
                }
            }
            methods.append(symbol)
        }
        var edgeColor = UIColor.black.cgColor   // color place holder
        if let name = methods.last{
            edgeColor = ColorCluster.shared.get(name)
        }
        borderColor = edgeColor
        borderWidth = 1
    }
}

Usage:

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {

       let cell = tableView.dequeue(for: PracticeDetailCell.self, ip: indexPath)
        cell.layer.debug()
        return cell
}

more code on github

I've had a look, and think it should be possible to trim down the ColorCluster-logic quite a bit, mostly by tightening up the get(: function a bit. Avoiding some if-elsing and only manipulating the index and count once.
struct ColorCluster {
    static var shared = ColorCluster()
    private init() { }

    private let colors: [UIColor] = [.blue, .magenta, .red, .green, .brown, .purple, .cyan]
    private var colorRegister = [String: Int]()
    private var count = 0

    mutating func getColor(for key: String) -> CGColor {
        let countIsOutOfBounds = count >= colors.count
        let nextAvailableIndex = countIsOutOfBounds ? 0 : count

        let index = colorRegister[key] ?? nextAvailableIndex
        count = countIsOutOfBounds ? 0 : count + 1

        return colors[index].cgColor
    }
}

I also had a quick look at the debug-extension. I haven't actually tested it, but the gist is to go down the functional route when creating the array of method-names:
extension CALayer {
    func debug() {
        // call stack trace, remove methods of the system
        let libs: Set<String> = ["UIKitCore", "QuartzCore", "CoreFoundation"]

        let methodNames = Thread.callStackSymbols.reduce([String]()) { names, symbol in
            let containingSymbol = libs.filter({ symbol.contains($0) })
            if containingSymbol.isEmpty {
                return names
            }
            return names + [symbol]
        }

        var edgeColor = UIColor.black.cgColor // color place holder
        if let name = methodNames.last {
            edgeColor = ColorCluster.shared.getColor(for: name)
        }
        borderColor = edgeColor
        borderWidth = 1
    }
}


Toggle logical NOT operator based on a boolean value I'm trying to think of a better way to write the below Javascript/NodeJS code without having to check the value of the boolean using an if statement. I know this can be accomplished with a ternary, but that would look ugly.

This is of course only an example, I'm sure there's a better way to write this; however I'm specifically wondering about this use-case.

var getNumbers = function(getNumbers) {
    return function(e) {
        if (getNumbers) {
            return !isNaN(e) ? e : null;
        } else {
            return isNaN(e) ? e : null;
        }
    };
};

var data = ['A', 'B', 'C', 'D', '1', '2', '3', '4'];

console.log(data.filter(getNumbers(true)));
console.log(data.filter(getNumbers(false)));

Output:

[ '1', '2', '3', '4' ]
[ 'A', 'B', 'C', 'D' ]

Using a function name as a variable name inside said function
This can be very confusing to someone reading your code(or you after having been away from your code for awhile)
Hither forth, getNumbers (the variable in the function) shall be referred to as showNumbers
XOR it baby
XOR is exactly what the current logic is doing anyway.
When showNumber is true invert other boolean value
//showNumber = true
T ^ showNumber == F
F ^ showNumber == T

Else leave as it was
//showNumber = false
T ^ showNumber == T
F ^ showNumber == F

var getNumbers = function(showNumbers) {
    return function(e) {
        return isNaN(e) ^ showNumbers ? e : null;
    };
};

Potential gotcha
Javascripts XOR(^) is bitwise NOT Logical


chmod all .htaccess files under all website dirs under public_html dir This is my code to chmod all .htaccess files under all website dirs under public_html dir.

The reason I use this code is to make sure WordPress plugins won't "freak out" and change my .htaccess file undesirably.

This is a shared server environment so I can't access htppd.conf instead, and must edit the different .htaccess files per each site, with the below code:

for dir in "$HOME"/public_html/*.{com,co.il}/; do
  if pushd "$dir"; then
    chmod 644 .htaccess
    popd
  fi
done 2>/dev/null

Instead of manually scanning each files and directory and testing if they are a directory using pushd, I would directly try to target .htaccess files using find:

find "$HOME"/public_html/*.{com,co.il} -name ".htaccess"

Then, to apply the proper permissions, you can use the -exec option of find:

find "$HOME"/public_html/*.{com,co.il} -name ".htaccess" -exec chmod 644 {} \;


C function that emulates string concatenation operator '+' in Python or Go I'm working on a project of mine which required multiple calls to strncat(), and it came to me that it'd have been much easier if C also had something like "file://" + getenv("HOME") + filename for string generation. So I built a function that works in a similar fashion, it is a variadic function, the arguments are NULL terminated.
Usage: char *str = mutant_string ("file://", getenv ("HOME"), filename, NULL);
It's obviously up to the user to free the memory str allocated.
The code:-
char *mutant_string (const char *s, ...) {

    va_list argv;
    va_start (argv, s);

    char *return_string = strdup (s);

    size_t realsize, nmemb;

    realsize = strlen (return_string); // Or initial size

    char *c, *ptr;

    while ((c = va_arg(argv, char*)))
    {
        nmemb = strlen (c);
        realsize += nmemb;

        ptr = realloc (return_string, sizeof(char) * (realsize + 1));

        if (!ptr) {
            perror ("realloc");
            free (return_string);
            va_end (argv);
            return NULL;
        }

        return_string = ptr;
        strncat (return_string, c, nmemb);
    }

    return_string[realsize] = 0;

    va_end (argv);

    return return_string;
}

I welcome any kind of observations or opinions. Let me know if there's an external library or function that does exactly this and something that I can read.
Thank you.

It looks like this code needs a couple of headers to be included before it will compile:
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>

It also only works on platforms that have strdup().  It would be easy to make it more portable.
If s is a null pointer, then we have undefined behaviour, but we haven't indicated that we don't allow a null for the first argument.
Why are we not testing whether strdup() returns a null pointer?
Well done for avoiding the common hazard with realloc().  Too many C programmers fail to deal properly with allocation failure.  The scope of ptr could be reduced so that we declare and initialise together, and we don't need to multiply by sizeof (char) (which must be 1, by definition):
char *ptr = realloc(return_string, realsize + 1);

The use of strncat() in a loop is wasteful, since it starts from the beginning every time.  Since we know where the string ends, it's better to start from that position.
The algorithm could be much more efficient.  Instead of many realloc() calls, we could make two passes over the arguments: one to measure the total size, and one to copy the characters across.
Compare this extract from a Valgrind run of the code:
==25527== HEAP SUMMARY:
==25527==     in use at exit: 0 bytes in 0 blocks
==25527==   total heap usage: 5 allocs, 5 frees, 1,063 bytes allocated

against the two-pass version that doesn't need to realloc:
==24899== HEAP SUMMARY:
==24899==     in use at exit: 0 bytes in 0 blocks
==24899==   total heap usage: 2 allocs, 2 frees, 1,037 bytes allocated

That's fewer than half the allocations for a very simple "Hello World" test program:
char *mutant_string (const char *s, ...);

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

int main(void)
{
    char *s = mutant_string("Hello", " ", "World", "!", NULL);
    if (!s) {
        return 1;
    }
    puts(s);
    free(s);
}

Here's what the two-pass version looks like:
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>

char *mutant_string(const char *s, ...)
{
    if (!s) {
        return NULL;
        /* alternatively - return a new, empty string */
    }

    va_list argv;

    /* First pass - measure the total length */
    size_t total_len = strlen(s);
    va_start(argv, s);
    const char *c;
    while ((c = va_arg(argv, const char*))) {
        total_len += strlen(c);
    }
    va_end(argv);

    char *buf = malloc(total_len + 1);
    if (!buf) {
        return buf;
    }

    /* Second pass - copy the string data */
    char *p = buf;
    {
        /* first argument */
        size_t len = strlen(s);
        memcpy(p, s, len);
        p += len;
    }

    /* other arguments */
    va_start(argv, s);
    while ((c = va_arg(argv, const char*))) {
        size_t len = strlen(c);
        memcpy(p, c, len);
        p += len;
    }
    va_end(argv);
    *p = '\0';

    return buf;
}


A more efficient mul-interval From 2.11

  Exercise 2.11.  In passing, Ben also
  cryptically comments: ``By testing the
  signs of the endpoints of the
  intervals, it is possible to break
  mul-interval into nine cases, only one
  of which requires more than two
  multiplications.'' Rewrite this
  procedure using Ben's suggestion.

I wrote the following:

(define (negative? . n) (or (= 0 (length n))
                            (and (> 0 (car n)) (apply negative? (cdr n)))))

(define (positive? . n) (or (= 0 (length n))
                            (and (>= (car n) 0) (apply positive? (cdr n)))))

(define (straddles-zero? x) (and (>= 0 (lower-bound x))
                                 (>= (upper-bound x) 0)))

(define (fast-mul-interval x y)
  (let* ((u-x (upper-bound x))
        (u-y (upper-bound y))
        (l-x (lower-bound x))
        (l-y (lower-bound y)))
    (cond
      ; same sign - max is neg, or min is pos
      ((positive? l-y l-x) (make-interval (* l-x l-y) (* u-x u-y)))
      ((negative? u-y u-x) (make-interval (* u-x u-y) (* l-x l-y)))
      ; x and y have opposite signs
      ((and (positive? l-x)
            (negative? u-y)) (make-interval (* u-x l-y) (* l-x u-y)))
      ((and (positive? l-y)
            (negative? u-x)) (make-interval (* l-x u-y) (* u-x l-y)))

      ; x straddles zero
      ((straddles-zero? x)
       (if (straddles-zero? y)
           (make-interval (min (* l-x u-y) (* l-y u-x))
                          (max (* l-x l-y) (* u-x u-y)))
           (if (negative? u-y)
               (make-interval (* u-x l-y) (* l-x l-y))
               (make-interval (* l-x u-y) (* u-x u-y)))))
      ((straddles-zero? y)
       (if (negative? u-x)
           (make-interval (* u-y l-x) (* l-y l-x))
           (make-interval (* l-y u-x) (* u-y u-x)))))))

What do you think?

The new version is here:

(define (fast-mul-interval x y)
  (let ((l-x (lower-bound x))
        (u-x (upper-bound x))
        (l-y (lower-bound y))
        (u-y (upper-bound y)))
    (cond ((> l-x 0)
           ;x > 0
           (cond ((> l-y 0)
                  ;y > 0
                  (make-interval (* l-x l-y) (* u-x u-y))
                  )
                 ((< u-y 0)
                  ;y < 0
                  (make-interval (* u-x l-y) (* l-x u-y))
                  )
                 (else
                  ;y contains 0
                  (make-interval (* u-x l-y) (* u-y u-x))
                  ))
           )
          ((> l-y 0)
           ;y > 0
           (if (< u-x 0)
               ; x < 0
               (make-interval (* u-y l-x) (* l-y u-x))
               ; x contains 0
               (make-interval (* l-x u-y) (* u-x l-y))
               )
           )
          ((< u-x 0)
           ;x < 0
           (if (< u-y 0)
               ; y < 0
               (make-interval (* u-x u-y) (* l-x l-y))
               ; y contains 0
               (make-interval (* u-y u-x) (* l-y u-x))
               )
           )
          ((< u-y 0)
           ;y < 0 and x contains 0
           (make-interval (* u-y u-x) (* l-x u-y))
           )
          (else
           ;x and y both contain 0
           (let ((p1 (* l-x l-y))
                 (p2 (* l-x u-y))
                 (p3 (* u-x u-y))
                 (p4 (* u-x l-y)))
             (make-interval (min p1 p2 p3 p4) (max p1 p2 p3 p4)))
           ))))

Your logic is correct.

You may use let instead of let* since the value of one binding does not depend on the value of another binding.

Since efficiency is important, you can cut back on the number of comparisons as well.  For example, if u-x is negative, then l-x is also negative and need not be tested.  For this reason, the functions negative?, positive? and straddles-zero? are not necessary.  Instead, you can simply use nested ifs:

(if (< u-x 0)
    ; x is in R < 0
    (if (< u-y 0)
        ; y is in R < 0
        ...
        (if (< l-y 0)
            ; y contains 0
            ...
            ; y is in R >= 0
            ... ))
    (if (< l-x 0)
        ; x contains 0
        ...
        ; x is in R >= 0
        ... ))


"Functionalizing" this tree walking method and cleaning it up I have a MessageTree class that represents a binary tree of negative/positive. I'm looking to improve my walkTree method and make it more functional and succinct. Right now the try/catch is kind of ugly.

The goal is to feed an array of "steps" (Array[Boolean]) that indicate whether to walk right or left through each node. If the number of steps exceeds the number of available children, it simply returns None.

Here's what I've got:

case class MessageTree(
  val value: String,
  val negative: Option[MessageTree] = None,
  val positive: Option[MessageTree] = None

) {

  /**
   * Walk the node using a boolean input.
   * @param step
   * @return
   */
  def walk(step: Boolean): Option[MessageTree] = {
    step match {
      case true => this.positive
      case false => this.negative
    }
  }
}

object MessageTree {

  /**
   * Walk the tree and retrieve a child using a sequence of boolean steps.
   * @param tree
   * @param steps
   * @return
   */
  def walkTree(tree: MessageTree, steps: Array[Boolean]): Option[MessageTree] = {
    var walks = 0
    var node: Option[MessageTree] = Some(tree)

    try {
      while (walks < steps.size) {
        node = node.get.walk(steps(walks))
        walks += 1
      }
    } catch {
      case e: Throwable =>
        node = None
    }

    node
  }
}

You have selected the right tool (case classes) but are not using it properly.  In functional programming, a tree is an Algebraic Data Type.  In Scala, these are constructed with case classes but this is done with a sealed base trait (or abstract class) and a set of case classes which represent the different states/components of the type.  Here is how yours might look:

  sealed abstract class CrockPotTree[+A]
  case object Empty[A] extends CrockPotTree[Nothing]
  case class Node[A](value: A, negative: CrockPotTree[A],
      positive: CrockPotTree[A]) extends CrockPotTree[A]

Now all I need do is create functions which can add to, prune or walk the tree.  These will do so recursively, using pattern matching to do the appropriate thing depending on the type of the node.  There is more than one way to do this, but first let me say a few things about the example code above:

I used a sealed abstract as the base.  This class can only be used within this module and not elsewhere.  This means code for walking/fetching/manipulating CrockPotTrees will not fail because somebody extended the class and added something crazy and unpredictable.  It is abstract because only the derivative case classes have meaningful state (Empty, not Empty etc).
I could have used a trait.  There are still different opinions about this.
I have made it generic: CrockPotTree[+A] rather than CrockPotTree[String], so it can hande all types of objects.  You can use it (once an apply method has been added to the companion object) to create a CrockPotTree[String] or CrockPotTree[Int] or whatever.
I have made it covariant (CrockPotTree[+A] rather than CrockPotTree[A]).  This opens the possibility that if you add a Bear value to a tree full of Wolf objects, you will get back a CrockPotTree[Animal] rather than a simple error.
There is no need for Option[CrockPotTree] because of the Empty crockpot type.  I will explain later.

Now, the functions that work should be recursive (or fold over a path through the tree, which is a kind of recursion).  Why?  Because this is a recursive data type.  The first point to consider about this is that your steps type in WalkTree is wrong.  It is almost certainly better to have it a list than an array.  That way, each recursive iteration of walkTree can look something like this:

  steps match {
      Case Nil => // Return Some(value) if this is a Node or Nothing if it is an Empty
      Case x :: xs => walkTree(walk(x), xs)
  }

Note how walkTree still returns Option[value] even though Option[CrockPotTree] is no longer part of the structure.

Using the list is both faster and simpler than using an array.  That said, the next important question is where to put these functions.  You have 2 basic choices:

In the companion object.  In this case, functions should use pattern matching to decide what is the appropriate option for the particular case class.
Abstract methods defined in the abstract class (where appropriate) and concrete implementations in the case classes.

(If you use a trait rather than abstract class, you also have the choice of making some functions concrete in the parent trait).

Option 1 is the functional approach as seen in functional languages which don't offer OO.  I've written tree types in Haskell code and that's just how it is done there.  Option 2 is more OO (pattern matching goes away because each case class knows how to deal with itself) while still being good functional code if you do it right.  Option 2 is the way most functions in the Scala collections library are done.

Ironically, your function is very much Option 1 style despite the fact that you put it into the class.  It could be moved into the companion object with almost no modification, because it doesn't use polymorphism properly.

Since I have already given an example of how walkTree would look in Option 1 style (which is how your code is effectively done), here's how some simple functions would be done Option 2 stylee:

  sealed abstract class CrockPotTree[+A] {
    def isEmpty: Boolean
    def size: Int
  }
  case object Empty[A] extends CrockPotTree[Nothing] {
    def isEmpty = true
    def size = 0
  }
  case class Node[A](value: A, negative: CrockPotTree[A],
      positive: CrockPotTree[A]) extends CrockPotTree[A] {
    def isEmpty = false
    def size = positive.size + negative.size + 1
    /* This implementation of size is *not* tail recursive
     * and could create a stack overflow.  There is a better
     * way to do it but this way is chosen for simplicity here
     */
  }

Now, despite my warning about the stack overflow risk, look at the size function.  It works because an empty tree will return a size of zero.  A non-empty tree will add 1 (for itself) to the combined sizes of its positive and negative trees.  I want to make two points about this:

You should now have enough knowledge to create an Option 2 style walk, if you think hard enough.
This is why we seal algebraic data types so they cannot be extended.  My size function knows that the answer will always be 0 or more, because there are only two CrockPotTree types and they both return a proper value.  If some idiot added an new case class which returned something else (null, for example), things could explode.

My final two observations...

Firstly, you should have an apply function in the companion object, so that you could create a CrockPotTree like this

  val cpt = CrockPotTree(values)

where values is some collection of booleans and values which CrockPotTree will know how to build into a tree.  the apply function would need to call an append function recursively.  In Option 2 style, Empty and Node classes would both have their own recursive append function.

Secondly, I am not at all sure that Boolean is an appropriate type here.  You might want to create your own binary type (if that really is appropriate for this tree structure).  Guess what?  It would also be an ADT (algebraic data type).

I leave you with some useful links:

https://gleichmann.wordpress.com/2011/01/30/functional-scala-algebraic-datatypes-enumerated-types/
https://gist.github.com/mbbx6spp/5548937
http://twitter.github.io/effectivescala/#Functional%20programming-Case%20classes%20as%20algebraic%20data%20types


File Writing optimizations - FileOutputStream vs Writers I'm using a simple logger on a server, which uses a method that looks like this:

@Override
public synchronized void write(String logtag, String reason, int severity) {

    long logtime = System.currentTimeMillis();
    String message = "";

    switch (severity) {
        case DEBUG:
            if (!debug) return;
            message = "[" + logtime + "] {DEBUG} " + logtag + "% " + reason + "\n$";
            break;
        case INFO:
            message = "[" + logtime + "] {INFO} " + logtag + "% " + reason + "\n$";
            break;
        case ERROR:
            message = "[" + logtime + "] {ERROR} " + logtag + "% " + reason + "\n$";
            break;
        case FATAL:
            message = "[" + logtime + "] {FATAL} " + logtag + "% " + reason + "\n$";
           break;
        case EXCEPTION:
            message = "[" + logtime + "] {EXCEPTION} " + logtag + "% " + reason + "\n$";
            break;
        default:
            message = "Anthrazit error! Invalid severity: " + severity;
    }

    try {
        out.write(message.getBytes(StandardCharsets.UTF_8));
    } catch (IOException e) {
        errorOnInit(e);
    }

    if (exitOnFatal && severity == FATAL) {
        System.err.println("Anthrazit exit on fatal: " + message + ". Please check the logs.");
        close();
        System.exit(1);
    }

}

After doing some research I found some classes designed to to this, like PrintWriter and others. Is there any performance difference between my approach and the PrintWriters?

According to its javadoc:

  FileOutputStream is meant for writing streams of raw bytes such as image data. For writing streams of characters, consider using FileWriter.

See also: https://stackoverflow.com/questions/11371154/outputstreamwriter-vs-filewriter

You could also use a StringBuilder instead of doing manually a string concatenation.

Also, the whole method is sychronized. The message probably does not need to be constructed in the sychronized block. Only concrete write to the Stream/File should be sychronized:

long logtime = System.currentTimeMillis();
// construct string etc

sychronized(this){
    out.write(message.getBytes(StandardCharsets.UTF_8));
}

You could also try to read the code of "real" logging frameworks to get ideas how they work.


Calculator for magic item creation in RPG I have created a simple calculator that will assist in creating magic items from the tabletop RPG Pathfinder. Here are the full requirements, and if you want a more concise version, here is a player generated summary of the process.

I have been trying to get into F# and functional programming in general. I come from a strong object-oriented background and it has been difficult in ignoring those habits. Please focus on whether or not this is idiomatic F#. I would love to hear if there is a better way to write any of it. And, if you find bug, I'd be happy to know that too.

Questions

A couple of things specifically that I feel could be better would be the DC calculation. I wasn't sure how to approach applying the rush function only if the item was rushed. I also tried finding a place where I could apply functional composition, but I think I needed all the intermediate steps that I stop on.

Code

let baseDC casterLevel =
    5 + casterLevel

let rush dc =
    dc + 5

let ignoreRequirements requirements dc =
    dc + (5 * requirements)

let getBuildPrice basePrice =
    basePrice * 0.5

let getTime basePrice =
    floor (8.0 * basePrice / 1000.0)

let getMinBonusForNoCursed dc =
    dc - 6

let buildDaysWhileAdventuring isRushed buildTime =
    let adventuringBuildTime = buildTime / 4.0
    floor (if isRushed then adventuringBuildTime else adventuringBuildTime * 2.0)

[<EntryPoint>]
let main argv =
    printfn "%A" argv

    let isRushed = true
    let casterLevel = 1
    let requirementsIgnored = 1

    let basePrice = 5000.0
    let buildPrice = getBuildPrice basePrice

    let buildTime = getTime basePrice
    let daysAdventuring = buildDaysWhileAdventuring isRushed buildTime

    let DC =
        match isRushed with
        | false -> baseDC casterLevel |> ignoreRequirements requirementsIgnored
        | true -> baseDC casterLevel |> ignoreRequirements requirementsIgnored |> rush

    let minDC = getMinBonusForNoCursed DC

    printfn "Min/DC: %i/%i Cost: %.2f Days/Hours: %.1f/%.1f" minDC DC
        buildPrice daysAdventuring buildTime

    0 // Return code

I see a few problems with this, most notably, you have a plethora of magic numbers.

let baseDC casterLevel =
    5 + casterLevel

let rush dc =
    dc + 5

let ignoreRequirements requirements dc =
    dc + (5 * requirements)

let getBuildPrice basePrice =
    basePrice * 0.5

let getTime basePrice =
    floor (8.0 * basePrice / 1000.0)

let getMinBonusForNoCursed dc =
    dc - 6

let buildDaysWhileAdventuring isRushed buildTime =
    let adventuringBuildTime = buildTime / 4.0
    floor (if isRushed then adventuringBuildTime else adventuringBuildTime * 2.0)

In order, I see:

5 + casterLevel: what is 5?
dc + 5: what is 5? Is it the same as #1?
5 * requirements: what is 5? Is it the same as #1 or #2?
basePrice * 0.5: what is 0.5? Is it 1 / 2 or something different?
8.0 * basePrice: what is 8.0?
#5 / 1000.0: what is 1000.0?
dc - 6: what is 6?
buildTime / 4.0: what is 4.0?
adventuringBuildTime * 2.0: what is 2.0, is it 4.0 / 2, is it the 4.0 from #8 / 2?

For a functional program, it's not too bad. I don't like how many times you use if (I always prefer match), but when dealing with boolean types it does quite fine.


Project Euler #9 in haskell I am trying to teach myself haskell by means of project Euler

i sovled problem #9 by means of this code which is on the brute force side I was wondering if there is a more elegant/haskell/efficient solution

let project_euler_9 = [(a,b,c) | c <- [1..500], b <- [1..c], a <- [1..b], a^2 + b^2 == c^2, a+b+c==1000 ]

As already mentioned there are formulas for enumerating the pythagorean triples, but your solution can be made quite fast by bounding the search space more. You have already taken care of the symmetry in the solutions by letting a be less then b (a <- [1..b]), but note that since a + b + c == 1000, once you know the value of a and b you know that c = 1000 - a - b so you only need to check that one case:

[(a,b,c) | a <- [1..500], b <- [1..a], let c = 1000 - a - b, a^2 + b^2 == c^2]

The above code runs in less then a second on my machine with GHCi.


Find max difference for a given array You are given an array of N integers, find the Max Difference where
  the index of largest number is greater than the index of the smallest
  number. If not find the next smallest number, so on and so forth. If
  this condition cannot be met, return -1

Sample Input                        Sample Output

values = [1, 2, 3, 4, 5]                  4

values = [2, 3, 4, 5, 1]                  3

values = [121, 30, 45, 55, 1]             -1

My solution, Python 2.7

def findNewIndex(max_index, min_index, input_array):
    input_array.pop(min_index)

    num = min(input_array)
    min_index = input_array.index(num)

    if max_index > min_index:
        return input_array[max_index] - input_array[min_index]
    elif max_index < min_index and input_array:
        return  findNewIndex(max_index, min_index, input_array)
    else:
        return -1

def maxDifference(input_array):
    max_num = max(input_array)
    max_index = input_array.index(max_num)

    min_num = min(input_array)
    min_index = input_array.index(min_num)

    if max_index > min_index:
        print input_array[max_index] - input_array[min_index]
    else:
       ans =  findNewIndex(max_index, min_index, input_array)
       print ans

I'd like feedback on the solution, is this a good way to solve it? Any improvements I can make to the code in terms of performance/style?

Overcomplicating

To start with, if what we're looking for is just the smallest number to the left of the largest number, we can make that algorithm more explicit:

def max_difference(xs):
    max_idx, max_elem = max(enumerate(xs), key=operator.itemgetter(1))

    if max_idx == 0:
        return -1
    return max_elem - min(xs[:max_idx])

Rather than making 4 passes to find the max and min elements and their respective indices, we can find the max element and its index in a single pass (taking advantage of enumerate and the fact that max takes a key function), and then simply searching the left side of that for the min element.

We don't need a findNewIndex-type method since once we know where the max element is and its index, that defines the domain for where the min element could be. We don't have to pop elements to find it.

A single pass

We can do better though. For each element, we can keep track of the smallest element we've seen so far and the largest element we've seen so far, and simply update as appropriate: the min element can get smaller, the max diff can only get bigger we see the new biggest:

def max_difference(xs):
    min_elem = xs[0]
    max_elem = xs[0]
    max_diff = -1

    for elem in xs[1:]:
        min_elem = min(elem, min_elem)
        if elem > max_elem:
            max_diff = max(max_diff, elem - min_elem)
            max_elem = elem

    return max_diff

Naming

The python style guide prefers snake_case to camelCase.


Super Reduced String Online challenge on Hacker Rank.

  Problem Statement

  Steve has a string, S, consisting of n lowercase English alphabetic >letters. In one operation, he can delete any pair of adjacent letters with same >value. For example, string "aabcc" would become either "aab" or "bcc" >after operation.

  Steve wants to reduce  as much as possible. To do this, he will repeat the >above operation as many times as it can be performed. Help Steve out by finding >and printing S's non-reducible form!

  Note: If the final string is empty, print Empty String.

  Input Format

  A single string, S.

  Constraints

  \$1 ≤ n ≤ 100\$

  Output Format

  If the final string is empty, print Empty String; otherwise, print the final >non-reducible string.

  Sample Input

  aaabccddd

  Sample Output

  abd

Solution Code

public class Solution {
    private static String solve(String input) {
        int len = input.length();

        int i = 0;
        while (i < len - 1) {
            char current = input.charAt(i);
            char next = input.charAt(i+1);

            if (current == next) {
                input = input.substring(0, i) + input.substring(i+2);
                len = input.length();
                i = 0;
                continue;
            }
            i++;
        }
        if (input.length() == 0) {
            return "Empty String";
        }
        return input;
    }

    public static void main(String[] args) {
        Scanner s = new Scanner(System.in);
        System.out.println(solve(s.next()));
    }
}

This is a brute-force solution, as I feel kind of confused when dealing with string algorithms. Hope for some good suggestions here.

Doc comments for public methods are indispensable.

I'd probably just use a foreach-loop and a DIY stack:

/** @return input stripped of all abutting identical characters */
static String solve(String input) {
    char []remains = new char[input.length()];
    int top = -1; // highest valid index

    for (char c: input.toCharArray())
        if (top < 0 || c != remains[top])
            remains[++top] = c;
        else
            --top;

    return top < 0 ? "Empty String"
        : new String(remains, 0, top+1);
}


Using GetTempFileName api function to check write access to a directory On windows, to find out if I have write access to a directory, I found that it is easiest to just create a file in that directory.

In order to do so, I found GetTempFileName() API function to get an almost(?) unique file name in the desired directory, then I was going to create a file with that name. so if everything goes well, I have the write access and everything is fine.

Then I found two behavior from the mentioned API call:

If I have write access, it creates the temporary file and returns the path through its output parameter, named lpTempFileName
When I don't have write access, it just clear out the buffer, and literally returns an empty string as temp file name (lpTempFileName).

I can't find this bit of information in documents. Is this always valid? If so I can rely on it and just decide based on if the returned string is empty or not, instead of calling CreateFile and Closing the Handle afterwards.

Here is what I'm doing now:

bool is_writable = true;

try
{
    CString tmp_file_name;
    ::GetTempFileName(folder_path, "ttt", 0, tmp_file_name.GetBuffer(MAX_PATH));
    HANDLE file = CreateFile(tmp_file_name, GENERIC_READ | GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
    if ( file == INVALID_HANDLE_VALUE )
    {
        is_writable = false;
    }
    else
    {
        CloseHandle( file );
        DeleteFile( tmp_file_name );
        is_writable = true;
    }
}
catch (...)
{
    is_writable = false;
}

But here is what I can do if the above assumption is correct:

bool is_writable = true;

try
{
    CString tmp_file_name;
    ::GetTempFileName(folder_path, "ttt", 0, tmp_file_name.GetBuffer(MAX_PATH));

    if ( tmp_file_name.IsEmpty() )
    {
        is_writable = false;
    }
    else
    {
        DeleteFile( tmp_file_name );
        is_writable = true;
    }
}
catch (...)
{
    is_writable = false;
}

Certainly, two less API calls.

So is my finding always stands correct? can I rely on the second snippet?

If so I can rely on it and just decide based on if the returned string is empty or not

No, you may use second version but do not check for string content. More about this later.

  On windows, to find out if I have write access to a directory,

Depends what you exactly mean with write access. Your method is not bad (especially if you do not need an high performance check) however note that you're checking for permissions to:

Read file/directory list for a given directory (FILE_LIST_DIRECTORY).
Create a file in a directory (FILE_ADD_FILE).

As you can imagine this is different from write data into a file. Permissions are a complex thing and in a complex secure scenario you may have, for example, these permissions:

Granted: create a new file.
Granted: full write data into an existing file you own.
Granted: append data into an existing file you're not owner of.
Denied: discover existing files.
Denied: overwrite any existing data.
Denied: delete files (it doesn't matter if you're owner or not).

I hope I highlighted what your problem is, your actual check will fail but you still have write permissions to that folder. What you actually have to do is:

Obtain information about file security using GetFileSecurity().
Obtain DACL information about that file (GetSecurityDescriptorDacl() and GetAclInformation()).
Enumerate ACEs of your ACL to check for required permission (using GetAce() to obtain ACCESS_ALLOWED_ACE to read ACCESS_MASK field). Do not forget to interpolate these data with GetSecurityDescriptorOwner() and GetSecurityDescriptorGroup().

Funny enough to do it you need READ_CONTROL permission but they actually coincides.

What to do then? If you do need a fast implementation you may do the above (checking/skipping if insufficient permissions) and then try to write a test file (not just create it). Note that you may not have DELETE permission to remove this test file (and it may be a problem in production environments). Note that directory traverse in advanced scenarios is much more complex than you may imagine (see also FILE_TRAVERSE and BYPASS_TRAVERSE_CHECKING stuff).

Fortunately there are some ATL helper classes to simplify all these things, you may want to read The Windows Access Control Model: Part 2, in particular Do I Have Access? section.

If what you need is a simple check and you do not have a complex security setup then you may go with your own code (also second version if directory list permission is granted). Just few notes about it:

After each call to CString.GetBuffer() you must call CString.ReleaseBuffer() before object is usable again. Do it just before you call tmp_file_name.IsEmpty().
You actually do not need to check tmp_file_name content (which is actually an implementation detail even subject to change). To answer your answer then yes you may (see later) use second version but check GetTempFileName() return value, if it's zero then it failed (and you may check with GetLastError() if it's an unauthorized access error). Do not rely on output parameter, its content in case of errors is undefined.
I do not see any reason your code will throw exceptions, a catch-all statement may hide other (and more subtle) issues than write permission. I'd remove it.
You may want to use your first solution also writing something to that file if to check if you have FILE_APPEND_DATA permission. If file creation and writing are separate things (in your scenario) then first solution is the only one working.