Makefile that places object files into an alternate directory (bin/) I'm trying to learn how to use makefiles, and this is my attempt at making a makefile that compiles source files in a source directory (src/) into object files that are created in a bin directory.

##########################################
#           Editable options             #
##########################################

# Compiler options
CC=g++
CFLAGS=-c -Wall
LDFLAGS=
EXECUTABLE_NAME=testes

# Folders
SRC=src
BIN=bin
OBJ=$(BIN)/obj

# Files
SOURCE_FILES=\
    main.cpp \
    test.cpp

##########################################
#    Don't touch anything below this     #
##########################################
OBJECT_FILES=$(addprefix $(OBJ)/, $(SOURCE_FILES:.cpp=.o))

build: create_directories create_executable
    @echo "Build successful!"

create_executable: create_objects
    @$(CC) $(LDFLAGS) $(OBJECT_FILES) -o $(BIN)/$(EXECUTABLE_NAME)
    @echo "Created executable."

create_objects: $(SOURCE_FILES)
    @echo "Created objects."

create_directories:
    @mkdir -p $(OBJ)

%.cpp:
    @echo "Compiling "$@
    @$(CC) $(LDFLAGS) -c $(SRC)/$@ -o $(OBJ)/$(patsubst %.cpp,%.o,$@)

clean:
    @rm -r -f $(BIN)

This makefile works, I just want to make sure I'm doing everything correctly.  Any changes you guys would suggest?

You are not using the make system properly.  As a result, it will always relink the executable, even when everything is already up to date.  On the other hand, it might fail to recompile some source files into object files.

When you run make…

It will try to create the build target.  No file named build exists, so…

It will try to create the prerequisites of build, namely create_directories and create_executable, neither of which are names of existing files.

In an attempt to create a file called create_directories, it runs mkdir -p $(OBJ).
For create_executable, it needs to rebuild create_objects.

For create_objects, it just verifies that all of the source files are present.

For any of the $(SOURCE_FILES) that is missing, it "creates" it using the %.cpp: rule, which actually tries to compile the missing .cpp into a .o.
Once all of the $(SOURCE_FILES) are present, it echoes "Created objects.".  That's a false claim due to the error above.

Having thus falsely satisfied the prerequisites for create_executable, it proceeds to attempt to link the executable, even if any of the $(OBJECT_FILES) is missing.

The main remedy is to specify real targets and real dependencies, rather than phony labels like create_executable.

Here is how I would write it.  Other than the major problems cited above, I have also noted many minor issues in the comments.

#########################################################
# Replacing everything that you told me not to touch... #
#########################################################
EXECUTABLE_FILES = $(EXECUTABLE_NAME:%=$(BIN)/%)
OBJECT_FILES     = $(SOURCE_FILES:%.cpp=$(OBJ)/%.o)
# ^^^ A more succinct expression for $(OBJECT_FILES), using
#     http://www.gnu.org/software/make/manual/make.html#Substitution-Refs

build: $(EXECUTABLE_FILES)

clean:
    rm -r -f $(BIN)
    @# ^^^ I don't recommend suppressing the echoing of the command using @

# http://www.gnu.org/software/make/manual/make.html#Phony-Targets
.PHONY: build clean

$(EXECUTABLE_FILES): $(OBJECT_FILES)
    @$(CC) $(LDFLAGS) -o $@ $^
    @# ^^^ http://www.gnu.org/software/make/manual/make.html#Automatic-Variables
    @echo "Build successful!"

# http://www.gnu.org/software/make/manual/make.html#Static-Pattern
$(OBJECT_FILES): $(OBJ)/%.o: %.cpp
    @echo Compiling $<
    @# ^^^ Your terminology is weird: you "compile a .cpp file" to create a .o file.
    @mkdir -p $(@D)
    @# ^^^ http://www.gnu.org/software/make/manual/make.html#index-_0024_0028_0040D_0029
    @$(CC) $(CFLAGS) -o $@ $<
    @# ^^^ Use $(CFLAGS), not $(LDFLAGS), when compiling.


Is "int2rgb" an OK method name? All colors can be represented by an int, and often you want to get the individual red, green and blue values from that int.

I have a method int2rgb (and another called rgb2int) that does this. Is this a clear/good name?

public static int[] int2rgb(final int color) {
    return new int[] { (color >> 16) & 0xFF, (color >> 8) & 0xFF, color & 0xFF };
}

public static int rgb2int(final int red, final int green, final int blue) {
    return (red << 16) + (green << 8) + blue;
}

public static int rgb2int(final int[] color) {
    return (color[0] << 16) + (color[1] << 8) + color[2];
}

And in case you are interested, the rest of the class is here.

As far as naming is concerned: Using 2 instead of to is pretty much a no-no - "to" is spelled out everywhere in the Java standard library. Further Java's naming convention is to capitalize every word, but the first in method names - except for acronyms which are all-caps no matter where they appear in the name. So the methods should be called intToRGB and RGBtoInt (to is lower case because it directly follows an acronym - compare for example Color.RGBtoHSB in the java standard library).

In regards to the code: I'd recommend representing your colors as Color objects rather than 3-element arrays. If you choose to do that, you don't have to define any of your methods, because that functionality already exists in the Color class.


Finding numbers whose product of the digits are perfect squares How can I decrease the running time and compile time of the following code?

  Given three integers A, B and K, you need to report the Kth number
  between A and B (both inclusive), satisfying the property that: the
  number does not contain a digit 'zero' and product of digits of number
  is a perfect square. If there doesn't exist that Kth number output
  "-1"(without quotes).

  Input:

  First Line contains the number of test cases \$T\$. Next \$T\$ lines contain 3
  space separated integers A, B, K as mentioned in the task.

  Output:

  Output \$T\$ lines as answer for every test case.

  Constraints:

  \$1 ≤ T ≤ 100\$
  \$1 ≤ A ≤ B ≤ 10^{18}\$
  \$1 ≤ K ≤ 10^7\$

  Sample input:

3
1 10 4
8 14 2
1 1000 23

  Output:

-1
11
119

public static void main(String[] args) {
    Scanner in = new Scanner(System.in);
    int cases = in.nextInt();
    long A,B,K;

    int caseNum =0;
    while(caseNum < cases)
    {
        A = in.nextLong();
        B = in.nextLong();
        K = in.nextLong();
        ArrayList<Long> products = new ArrayList<>();
        long product = 1;
        for(long i=A,k=0;i<=B && k<(B-A+1);i++,k++)
        {
            long num = i;

            double sqrt;

            ArrayList<Long> temp = new ArrayList<>();
            while(num>0)
            {
                temp.add(num%10);
                num = num/10;
            }
            for(int j=0;j<temp.size();j++)
            {
                if(temp.get(j)==0){
                    product=-1;
                    break;

                }
                product = product*temp.get(j);
                //System.out.println("Currently processing: "+ temp.get(j));

            }
            sqrt = Math.sqrt(product);

            if( product%sqrt==0)
            {   products.add(i);

            }
            product=1;
        }
        try{
            System.out.println(products.get((int)K-1));
        }catch (Exception e) {
            // TODO: handle exception
            System.out.println(-1);
        }

        caseNum++;
    }
}
}

Compile time really shouldn't be an issue. Right now, I would focus on readability, and then maybe runtime performance.

Variable names

Normally, I really dislike short variable names. In this case it's ok for A, B, and K, because they are part of the question. But if you do use the same variable names as in the question, you should probably do it consistently: cases should be T.

For the other variable names, you have no excuses: temp isn't a good name, and caseNum vs cases is also somewhat confusing. The worst is probably k, as you already have K.

Perform Action on Number

Instead of num = num/10; you can write num /= 10, instead of product=-1; you could write product--, and instead of product = product*temp.get(j); it could be product *= temp.get(j);. Your way isn't necessarily bad, but the alternatives are more common and easier to read.

Declare variables where they are used

You are declaring double sqrt; a lot earlier than you need sqrt, which makes your code harder to read. Same for product.

For loop with two indices

This for loop: for(long i=A,k=0;i<=B && k<(B-A+1);i++,k++) is extremely hard to understand.

Luckily, you can just remove k. The stop condition is already covered by i.

Enhanced for loops

If you use for (Long currentTemp : temp) { you can get rid of the j variable.

Functions and Tests

main really should only be responsible for starting your program, nothing else. Extract the code to a separate function, that way, you can also easily test it. To use unit tests, it would also be useful if you returned a result from a function instead of printing it directly.

You should also extract other functionality to its own function. For example, everything inside the while loop could go in its own function.

Different Approach

When getting a question like this, I wouldn't put everything in one big method. If you read the question, it becomes obvious that you need something to create the product of digits. So create a product method. You also need something to check if a number is a perfect square, so create that method as well (isPerfectSquare). Lastly, create a method to check if a number contains zero (containsZero).

Then, just create the main method and combine everything:

public static long run(long A, long B, long K) {
    int satisfiedConditionCount = 0;
    for (long i = A; i <= B; i++) {
        if (!containsZero(i) && isPerfectSquare(product(i))) {
            satisfiedConditionCount++;
            if (satisfiedConditionCount == K) {
                return i;
            }
        }
    }
    return -1;
}

public static long product(long number) {
    long product = 1;
    while (number != 0) {
        number /= 10;
        product *= number % 10;
    }
    return product;
}

public static boolean isPerfectSquare(long number) {
    int sqrt = (int) Math.sqrt(number);
    return sqrt * sqrt == number;
}

public static boolean containsZero(long number) {
    return String.valueOf(number).contains("0");
}

Now the code is a lot more readable, and because it is in separate functions, you could profile it to find bottle necks, and then improve on those.

Another added benefit of separate functions (apart from the readability, reusability, and profiling) is that you can test them separately.


PHP MySQLi Prepared Statements: Can this select query be hacked/injected? i want to know can this be hacked/injected?

$stmt = $mysqli->prepare("SELECT * FROM myTable WHERE name = ?");
$stmt->bind_param("s", $_POST['name']);
$stmt->execute();
$result = $stmt->get_result();
if($result->num_rows === 0) exit('No rows');
while($row = $result->fetch_assoc()) {
  //do some stuff
}
var_export($ages);
$stmt->close();

Given an answer on Stack Overflow suggests almost identical code for protection, let alone using exactly the same principle you can safely assume that your query is protected.

If you want to know how it works, I also wrote an answer on Stack Overflow, https://stackoverflow.com/a/8265319/285587

Nevertheless, as this site is for the code reviews offering some suggestions, I would suggest to use PDO for database interactions instead of mysqli. Simply because PDO API is much more versatile and easier to use. see your snippet rewritten in PDO:

$stmt = $mysqli->prepare("SELECT * FROM myTable WHERE name = ?");
$stmt->execute([$_POST['name']]);
if($stmt->rowCount() === 0) exit('No rows');
while($row = $stmt->fetch_assoc()) {
  //do some stuff
}

as you can see some nagging operations are just gone. I wrote a tutorial on PDO, which I would quite expectedly recommend.


Prefix and suffix detection algorithm I have written an algorithm intended to find prefixes and suffixes in arrays of strings.

I would like to get opinions/suggestions/reviews on the code I wrote (specially cases in which my code would fail), but also, I would like to know if this algorithm can be rewritten in a more optimal way since I will be running this code against lots of entries.

Finding preffixes

function findPrefix(strings){
    if(!strings || strings.length === 0){
        return null;
    }

    var prefix = '',
        characters = strings[0].split(''),
        i, j;

    for(i = 0; i < characters.length; i++){
        var isPrefix = true,
            character = characters[i];

        for(j = 0; j < strings.length; j++) {
            var string = strings[j];

            isPrefix = isPrefix &&
                (string.length >= i + 1 && string[i] === character);
        }

        if(isPrefix) {
            prefix += character;
        } else {
            return prefix;
        }
    }
}

Finding suffixes

function reverse(string){
    return string.split('').reverse().join('');
}

function findSuffix(strings){
    if(!strings || strings.length === 0){
        return null;
    }

    return reverse(findPrefix(strings.map(reverse)));
}

A running example can be found here: http://jsbin.com/razuta/2/edit (click JavaScript and console to see the results)

As Martin R said, if all strings must match a common prefix, then that prefix is whatever the first and last string in a sorted array have in common.

The code in the SO answer isn't too pretty though. Here's my take (rags-to-riches?):

function findPrefix(strings) {
  if(!strings.length) {
    return ""; // or null or undefined; your choice
  }

  var sorted = strings.slice(0).sort(), // copy the array before sorting!
      string1 = sorted[0],
      string2 = sorted[sorted.length-1],
      i = 0,
      l = Math.min(string1.length, string2.length);

  while(i < l && string1[i] === string2[i]) {
    i++;
  }

  return string1.slice(0, i);
}

Compared to the SO answer, the main point here (besides style and camelCased variables) is that the array is copied, and the copy is sorted. Otherwise, the same array object that was passed to the function will be sorted as a side-effect.

Compared to your code, I've skipped the strings-it-truth'y check at the very start. I prefer to assume that arguments are at least the correct type (i.e. an array) - otherwise it's the caller's problem.

As for your current code:

As mentioned in the comments, you can use string.charAt(i) or simply string[i] (though the latter is a more recent addition to JS) instead of splitting the string into an array.
Your inner loop starts at index 0 - but that's the same string you're already using as your characters array, so you already know that it's going to match.
The moment you hit a non-match, use break to skip the rest of the loop. My bad, you're already returning from the loop, which has the same effect.

As for finding suffixes, your current code seems fine to me if you use the "sort-and-compare" approach above for findPrefix. Otherwise, it'd probably be infinitesimally faster to write a separate findSuffix function that just has its loops reversed. The logic's the same, the indices just run backward.

While nothing beats the sort-and-compare approach (except a better one; see below), here's, just for fun, another way to to use all strings, unsorted, to find the prefix:

function findPrefix(strings) {
  function commonPrefix(a, b) {
    var i = 0,
        l = Math.min(a.length, b.length);

    while(i < l && a[i] === b[i]) {
      i++;
    }
    return a.slice(0, i);
  }

  return strings.reduce(function (prefix, string) {
    // if prefix is empty or matches the string,
    // then just return the prefix immediately
    if(string.indexOf(prefix) === 0) {
      return prefix;
    }

    // otherwise, find the intersection between
    // prefix and the string
    return commonPrefix(prefix, string);
  });
}

Update: As MatinR pointed out in the comments, sort may be a drag on performance, and besides it isn't necessary, as you only need what would be the first and last string in a sorted array. These strings can alternative be found like so:

function extremes(strings) {
  // grab two strings to use as our initial guesses
  var initial = {
    largest: strings[0],
    smallest: strings[strings.length-1]
  };

  // loop through to find the extremes
  return strings.reduce(function (memo, string) {
    // slightly funky syntax, but it's short.
    (memo.largest > string) || (memo.largest = string);
    (memo.smallest < string) || (memo.smallest = string);
    return memo;
  }, initial);
}

The two strings in the returned object may then be used to determine the shared prefix (if any). The entire thing can also be built into findPrefixes, of course, rather than be a separate function as shown below:

function findPrefix(strings) {
  if(!strings.length) {
    return ""; // or null or undefined; your choice
  }

  var string1 = strings[0],
      string2 = strings[strings.length-1],
      i, l;

  // note: a regular for-loop is faster than forEach
  // I'm using forEach because it's prettier
  strings.forEach(function (string) {
    (string1 < string) || (string1 = string);
    (string2 > string) || (string2 = string);
  });

  i = 0;
  l = Math.min(string1.length, string2.length);
  while(i < l && string1[i] === string2[i]) {
    i++;
  }

  return string1.slice(0, i);
}


C++ int_cast function for checked casts? In order to detect run-time integer casting problems, I've created this function*:

template<typename TTo, typename TFrom>
static inline TTo int_cast(TFrom value)
{
    TTo result = static_cast<TTo>(value);
    if (static_cast<TFrom>(result) != value || (result >= TTo()) != (value >= TFrom()))
    {
        throw std::out_of_range("numeric_cast: value out of range");
    }
    return result;
}

Two questions:

Are there any loopholes in this function I have not accounted for (e.g. undefined behavior)?
What would be the best way to extend this to perform double-to-float and floating-to-integral conversions?

*Note: Yes, I realize Boost has a function like this. But I want my own. :)

Are there any loopholes in this function I have not accounted for (e.g. undefined behavior)?

No. Static cast will not result in undefined behavior.
If it compiles then the cast is good (assuming integer/float types).

Though some of your cast may potentially result in implementation defined behavior (depending on the sign-dness and size of the types).

signed x = int_cast<signed,unsigned>(-1); // implementation defined behavior.

Since this is only supposed to work on int(s). Then I would make it be a compile time error if you tried to use it with other types. You may also be able to do some compile time range checking when the inputs are literals.

In the following:

(result >= TTo()) != (value >= TFrom())

I am assuming this is some form of signdness test (to make sure both source and destination have the same sign). If you are going to do this please comment the code explaining exactly how you think it works.

There are integer traits that allow you to pull the signdess and size of the template type from the input/output types. You could potentially look at this to help identify things.


Delphi 2009 interface I have this code and at the end of it, FastMM shows that there were memory leaks with all the objects of TSinc:

unit sinc_int;

interface

uses
  Classes,
  log_listener_list_int,
  tick_listener_list_int;

type
  ISinc = interface;

  ISincList = interface;

  ISinc = interface ['{53814230-6308-4259-99FF-6054F2C91F9C}']
  function Execute: Boolean;
  function SincList: ISincList;
  function Log: string;
  procedure AddLog(const Texto: string);
  function LogListenerList: ILogListenerList;
  function TickListenerList: ITickListenerList;
  function SincIntervaloMili: Integer;
  function EstaSincronizando: Boolean;
  procedure Start;
  procedure Stop;
  end;

  ISincList = interface ['{F3355798-4584-4DCE-8EBA-3692B14FA132}']
  function Add(const Item: ISinc): Integer;
  function Count: Integer;
  function Get(const Index: Integer): ISinc;
  procedure Put(const Index: Integer; Item: ISinc);
  procedure Remove(const Item: ISinc);
  procedure Clear;
  procedure Start;
  procedure Stop;
  end;

implementation

end.

unit sinc_class;

interface

uses
  sinc_int, log_listener_list_int, tick_listener_list_int, sinc_thread_class;

type
  TSinc = class (TInterfacedObject, ISinc)
  protected
    FLog: string;
    FSincList: ISincList;
    FLogListenerList: ILogListenerList;
    FTickListenerList: ITickListenerList;
    FSincIntervaloMili: Integer;
    FEstaSincronizando: Boolean;
    FThread: TSincThread;
  public
    constructor Create; virtual;
    destructor Destroy; override;
    function Execute: Boolean; virtual; abstract;
    function SincList: ISincList;
    function Log: string; virtual;
    procedure AddLog(const Texto: string);
    function LogListenerList: ILogListenerList;
    function TickListenerList: ITickListenerList;
    function SincIntervaloMili: Integer;
    function EstaSincronizando: Boolean;
    procedure Start;
    procedure Stop;
  end;

implementation

uses
  sinc_list_class,
  log_listener_list_class,
  tick_listener_list_class;

{ TSinc }

procedure TSinc.AddLog(const Texto: string);
begin
  FLogListenerList.AddLog(Texto);
  FLog:=FLog+Texto;
end;

constructor TSinc.Create;
begin
  FSincList:=TSincList.Create;
  FLogListenerList:=TLogListenerList.Create;
  FTickListenerList:=TTickListenerList.Create;
  FEstaSincronizando:=False;
  FThread:=TSincThread.Create(Self);
end;

destructor TSinc.Destroy;
begin
  FSincList.Clear;
  FSincList:=nil;
  FLogListenerList.Clear;
  FLogListenerList:=nil;
  FTickListenerList.Clear;
  FTickListenerList:=nil;
  FThread.Free;
  inherited;
end;

function TSinc.EstaSincronizando: Boolean;
begin
  Result:=FEstaSincronizando;
end;

function TSinc.Log: string;
begin
  Result:=FLog;
end;

function TSinc.LogListenerList: ILogListenerList;
begin
  Result:=FLogListenerList;
end;

function TSinc.SincIntervaloMili: Integer;
begin
  Result:=FSincIntervaloMili;
end;

function TSinc.SincList: ISincList;
begin
  Result:=FSincList;
end;

procedure TSinc.Start;
begin
  FEstaSincronizando:=True;
  FSincList.Start;
  FThread.Resume;
end;

procedure TSinc.Stop;
begin
  FThread.Suspend;
  FSincList.Stop;
  FEstaSincronizando:=False;
end;

function TSinc.TickListenerList: ITickListenerList;
begin
  Result:=FTickListenerList;
end;

end.

And when I do this:

procedure TForm2.btnSinc1Click(Sender: TObject);
begin
  SincTest:=TSincTest.New;
  SincTest:=nil;
end;

It shows that there were memory leaks with the TSincTest class, its lists and even the thread.

I suggest to check around TSync as it inherits from TInterfacedObject, which is reference counted, a typical source of memory leak.


Place try/catch in business logic or user interface I found the following two types of exception-handling in Business Logic Layer.

ASP.NET 3.5 Enterprise Application Development uses a similar method like the first one (I read it few years ago).

I also found this on Stack Overflow, but it doesn't answer my question.

I'm wondering which one is better design and efficiency.

Method 1 - in Business Logic Layer

private int InsertUser(string firstname, string lastname, ref List<string> errors)
{
    if (!string.IsNullOrEmpty(firstname))
        errors.Add("First name is required.");

    if (!string.IsNullOrEmpty(lastname))
        errors.Add("Last name is required.");

    if (errors.Count > 0)
        return -1;

    int userId = -1;

    try
    {
        // Insert user and return userId
    }
    catch (Exception ex)
    {
        // Log error to database
        errors.Add("Error occurs. Please contact customer service.");
    }

    return userId;
}

Method 2 - in Business Logic Layer

private int InsertUser(string firstname, string lastname)
{
    if (!string.IsNullOrEmpty(firstname))
        throw new ArgumentNullException(firstname);

    if (!string.IsNullOrEmpty(lastname))
        throw new ArgumentNullException(lastname);

    int userId = -1;

    // Insert user and return userId. Let user handle the exception in UI.

    return userId;
}

The disadvantage of Method 2 is that the UI has to filter out what to display/not display the error depending on the exception.  For instance, you might not want to display a System.Data.Entity exception to the user.

It can be interesting to convey more information about the error to the end user, but it doesn't mean the UI layer has to be the only one to handle the exception.

You can both log the error into your database from the business layer and relay it to the UI layer if you rethrow the exception:

try {
    // Insert user and return userId...
} catch (Exception ex) {
    // Log error to database...
    throw;  // Relay error to UI layer.
}


Search table with filter in Jquery This is my code:

function search() {
    $("#myInput").keyup(function () {
        var value = this.value;

        $("table").find("tr").each(function (index) {
            if (index === 0) return;
            var id = $(this).find("td").first().text();
            $(this).toggle(id.indexOf(value) !== -1);
        });
    });
}

$('body').on('click', '#searchFonts', function () {
    var value = "Font"
    $("table").find("tr").each(function (index) {
        if (index === 0) return;
        var id = $(this).find("td").first().text();
        $(this).toggle(id.indexOf(value) !== -1);
    });
});

$('body').on('click', '#searchScripts ', function () {
    var value = "script"
    $("table").find("tr").each(function (index) {
        if (index === 0) return;
        var id = $(this).find("td").first().text();
        $(this).toggle(id.indexOf(value) !== -1);
    });
});

I'm looking for ways to make this run more effectively, as I'm sure there is a more efficient way to search for the specific value's seen below such as 'Font' and 'Script'.

D.R.Y.

There is a widely accepted principle amongst developers: Don't Repeat Yourself. The following block appears three times (with varying spacing):

$("table").find("tr").each(function(index) {
    if (index === 0) return;
    var id = $(this).find("td").first().text();
    $(this).toggle(id.indexOf(value) !== -1);
});

That can be abstracted into a function (which could accept a parameter for the value) which could then be called in place of that repeated block.

For example:

function toggleRowsWithKeyword(value) {
    $("table").find("tr").each(function(index) {
        if (index === 0) return;
        var id = $(this).find("td").first().text();
        $(this).toggle(id.indexOf(value) !== -1);
    });
}

And usage:

function search() {
  $("#myInput").keyup(function() {
      toggleRowsWithKeyword(this.value);
  });
}
$('body').on('click','#searchFonts',function(){
    toggleRowsWithKeyword("Font");
});
$('body').on('click','#searchScripts ',function(){
    toggleRowsWithKeyword("script");
});

Cache DOM lookups

Even with abstracting that code into a function, it would still be performing a query on the DOM.

$("table").find("tr")

That can be stored in a variable (or constant if ES-6's const is used):

var rows = $("table").find("tr");

To be on the safe side, that assignment can be placed inside a DOM-loaded callback (e.g. a function wrapped to the jQuery callback wrapper: $()):

$(function() {  //DOM is ready
    var rows = $("table").find("tr");
    //Define  toggleRowsWithKeyword(), add click handlers, etc.
});

Then the function can utilize rows instead of querying for the rows:

rows.each(function(index) {...

CSS Selector for all except first row

The CSS Selector:

var rows = $("table").find("tr")

Could be simplified to:

var rows = $("tr")

And the check for the first row (i.e. if (index === 0) return;) could be eliminated using the :not pseudo-class along with :first-child:

var rows =  $("tr:not(:first-child)")

More tips in article

For more tips about improving Javascript that interacts with the DOM, see this post: Stop Writing Slow Javascript. I know it bashes jQuery initially but it has some very helpful tips (and nice quotes).


Converting a wrapper object to a value object I have ChannelWrapper and Channel Value Object. I want to convert each and every field of ChannelWrapper to Channel Value Object.

public class ChannelWrapper extends BaseWrapper implements APIWrapper<Channel>,
        APIUnwrapper<Channel> {

    @XmlElement
    private Long id;

    @XmlElement
    private User user;

    @XmlElement
    private Tenant tenant;

// Getter Setters
    @Override
    public Channel unwrap(HttpServletRequest arg0, ApplicationContext arg1) {
    // TODO Auto-generated method stub
    return null;
    }

    @Override
    public void wrapDetails(Channel model, HttpServletRequest request) {

    this.id = model.getId();
    this.user = model.getUser();
    this.tenant = model.getTenant();

    }

    @Override
    public void wrapSummary(Channel model, HttpServletRequest request) {
    wrapDetails(model, request);

}

Channel Value Object:

public class Channel {

    private Long id;
    private User user;
    private Tenant tenant;
    //getter setters   }

Converter:

Channel channel = new Channel();
channel.setId(channelWrapper.getId());
channel.setUser(channelWrapper.getUser());
channel.setTenant(tenant);

Is this the best way to create a converter?

Any other general comments are appreciated.

Since both classes have the same getters and setters this can most easily be achieved using apache commons beanutils:

Channel channel = new Channel();
BeanUtils.copyProperties(channel, channelWrapper);

if you have more complex requirements (e.g. different property names or types), I would recommend dozer.


How could this be written using continuation passing style? Is there a way to use CPS in this code to make the two insert functions tail-call optimized?

defmodule BTree do

  defstruct tree: {node: nil, left: nil, right: nil}
  def new(element), do: %BTree{tree: {node: element, left: nil, right: nil}}

  #insert when there's some data in the tree
  def insert(%BTree{tree: {node: e, left: l, right: r}}, element) when e > element do
    newl = if l == nil, do: new(element), else: insert(l, element)
    %BTree{tree: {node: e, left: newl, right: r }}
  end

  def insert(%BTree{tree: {node: e, left: l, right: r}}, element) when e < element do
    newr = if r == nil, do: new(element), else: insert(r, element)
    %BTree{tree: {node: e, left: l, right: newr }}
  end

  def height(%BTree{tree: {node: _, left: l, right: r}}) do
    lh = if l != nil, do: height(l), else: 0
    rh = if r != nil, do: height(r), else: 0
    1 + max(lh, rh)
  end

end

#BTree.new(10) |> BTree.insert(5) |> BTree.insert(15) |> BTree.height()

As always any comments on idiomatic Elixir code and style are welcome as well.

I would like to slightly rewrite the problem before making it continuation based because it will make it clearer and avoid duplication:

defmodule BTree do
  defstruct tree: nil
  def new(e), do: %BTree{tree: {e, nil, nil}}

  def insert(%BTree{tree: root}, element) do
    %BTree{tree: do_insert(root, element)}
  end

  defp do_insert(nil, element) do
    {element, nil, nil}
  end

  defp do_insert({e, l, r}, element) when e > element do
    {e, do_insert(l, element), r}
  end

  defp do_insert({e, l, r}, element) when e < element do
    {e, l, do_insert(r, element)}
  end

  defp do_insert({_, _, _} = tuple, _element) do
    tuple
  end
end

BTree.new(10) |> BTree.insert(5) |> BTree.insert(15) |> IO.inspect

The previous code was generating a BTree for every node. Instead, we simply treat each node as a tuple with three elements (the node item, left and right). When inserting an element, we traverse the tree until we reach nil. We can make it tail recursive by passing a function and invoking the previous one at each step:

defmodule BTree do
  defstruct tree: nil
  def new(e), do: %BTree{tree: {e, nil, nil}}

  def insert(%BTree{tree: root}, element) do
    do_insert(root, element, &%BTree{tree: &1})
  end

  defp do_insert(nil, element, fun) do
    fun.({element, nil, nil})
  end

  defp do_insert({e, l, r}, element, fun) when e > element do
    do_insert(l, element, &fun.({e, &1, r}))
  end

  defp do_insert({e, l, r}, element, fun) when e < element do
    do_insert(r, element, &fun.({e, l, &1}))
  end

  defp do_insert({_, _, _} = tuple, _element, fun) do
    fun.(tuple)
  end
end

BTree.new(10) |> BTree.insert(5) |> BTree.insert(15) |> IO.inspect

Note though I find this code much harder to understand and I don't think you would get any benefit in practice.


Reverse the character order of the words in a string I've been trying to learn Java and one way I've been teaching myself is by tackling "programming challenge questions".

One such question is "reverse the character order of words in a string".

For example, " I   have a hat   " => " I   evah a tah   " (note the spaces). In my implementation, I don't deal with punctuation, so "I have a hat!" => "I evah a !tah".

I've written an implementation and I'd like some feedback, particularly regarding the readability and efficiency of the code (basically, is there a slicker implementation of what I'm trying to accomplish - to me, the answer is yes, especially the last part of my implementation).

The implementation is basically doing the following:

Create a one element-sized String array called candidateReversedWordsString and initialize it with the input String
Create a StringBuilder that will build all the reversed words.
Iterate through the characters in the string
If the character is not a space and the StringBuilder is empty, note the character index value in the wordStartIndex variable and add the character to the StringBuilder.
If the character is not a space and the StringBuilder is not empty, add the character to the StringBuilder
If the character is a space and the StringBuilder is not empty, take the only element in the candidateReversedWordsString array and basically concatenate the element with the reversed word from the StringBuilder using substrings.
If at the final character and the StringBuilder is not empty, slight adjustment to the concatenation

public String reverseWordsInString(final String string) {
    final String[] candidateReversedWordsString = new String[1];
    candidateReversedWordsString[0] = string;
    int stringCharacterIndex = 0;
    int wordStartIndex = 0;
    final StringBuilder reverseWordStringBuilder = new StringBuilder();
    while (stringCharacterIndex < string.length()) {
        if (' ' != string.charAt(stringCharacterIndex)) {
            if (reverseWordStringBuilder.length() == 0) {
                wordStartIndex = stringCharacterIndex;
            }
            reverseWordStringBuilder.append(string.charAt(stringCharacterIndex));
        }

        if (' ' == string.charAt(stringCharacterIndex) && reverseWordStringBuilder.length() > 0) {
            candidateReversedWordsString[0] = new StringBuilder().
                    append(candidateReversedWordsString[0].substring(0, wordStartIndex)).
                    append(reverseWordStringBuilder.reverse().toString()).
                    append(candidateReversedWordsString[0].substring(stringCharacterIndex)).toString();
            reverseWordStringBuilder.setLength(0);
        }

        if (stringCharacterIndex == string.length() - 1 && reverseWordStringBuilder.length() > 0) {
            candidateReversedWordsString[0] = new StringBuilder().
                    append(candidateReversedWordsString[0].substring(0, wordStartIndex)).
                    append(reverseWordStringBuilder.reverse().toString()).toString();
        }
        stringCharacterIndex++;
    }

    return candidateReversedWordsString[0];
}

I like the effort that you put into naming your variables.  You may have gone a bit overboard, though.  For example, instead of candidateReversedWordsString, how about result?

A few mechanical issues:

This could be a static method, since it relies on no instance state.
It's pretty funny that you defined a final String[] of length 1 to store one String with the ability to change its only element.  Why not just use a regular String variable?
You have…

 int stringCharacterIndex = 0;
 …
 while (stringCharacterIndex < string.length()) {
     …
     stringCharacterIndex++;
 }

That pattern would be easier to understand if written as

for (int i; i < string.length(); i++) {
    …
}

Here, I would opt for a short variable name i, which has the connotation of being a loop counter that is an index.  string[i] is easy enough to understand.

This statement summarizes your strategic error:

candidateReversedWordsString[0] = new StringBuilder().
        append(candidateReversedWordsString[0].substring(0, wordStartIndex)).
        append(reverseWordStringBuilder.reverse().toString()).
        append(candidateReversedWordsString[0].substring(stringCharacterIndex)).toString();

Basically, you are rebuilding the entire tentative result string every time you want to reverse a word.  That's bad for performance, and also complicates your code.

I think that you would be better off without using any StringBuilder, and just using a char[] instead, especially since you know that the result will be exactly the same length as the input.

Consider this solution instead:

public static String reverseWords(String string) {
    char[] c = string.toCharArray();
    int wordStartIndex = -1;
    for (int i = 0; i < c.length; i++) {
        if (c[i] == ' ') {
            // Ignore spaces
            continue;
        }
        if (wordStartIndex < 0) {
            // Mark start of word
            wordStartIndex = i;
        }
        if (i + 1 == c.length || c[i + 1] == ' ') {
            // Word ends here; reverse it
            for (int a = wordStartIndex, b = i; a < b; a++, b--) {
                char swap = c[a];
                c[a] = c[b];
                c[b] = swap;
            }
            wordStartIndex = -1;
        }
    }
    return new String(c);
}


SFML Pong Game Performance I was hoping I could get some feedback on performance of my animations overall. It could just be me but I keep getting a bit of lag despite being at 60FPS constantly.

Objects on screen seem to tear a little bit.

Here's my code in full.

Here is my game loop:

//clocks and times used to get custom game loop working
    sf::Clock clock;
    sf::Time timeSinceLastUpdate = sf::Time::Zero;

    //setting g_GameState to be intro on run
    g_GameState = 0;

            //main loop to run the entire length of games
            //life
            while (g_Window.isOpen())
            {
                sf::Time dt = clock.restart();
                timeSinceLastUpdate += dt;

                while(timeSinceLastUpdate > TIME_PER_FRAME)
                {

                    timeSinceLastUpdate -= TIME_PER_FRAME;
                    processEvents();
                    update(TIME_PER_FRAME);

                }
                updateFPSCounter(dt);
                render();
            }

It's this performance issue I'd like any feedback or advice on.

Also, if anyone thinks I could do collision detection better as well, could you give me any pointers?

For the player paddles I use this to update their movement:

 //Not using deceleration
    //so setting mVelocity as 0 each time
    mVelocity.x = 0;
    mVelocity.y = 0;

    //Handle if player keys are pressed to move up
    //or down
    if(mIsMovingUp){
        mVelocity.y = -mSpeed;
    }
    else if(mIsMovingDown){
        mVelocity.y = mSpeed;
    }

    //move the play based on current mVelocity size
    this->move(mVelocity * elapsedTime.asSeconds());

First suggestion: clean up your code. There are a ton of inconsistencies in variable names, indentation, brackets, defines ...

Variables

I was confused when seeing g_ variables in your game.h file, suggesting that these were global variables yet they are declared as member variables. My guess is that you moved away from global variables (which is a good idea) but forgot to fix the naming. I know it's annoying to change all those names now, but it'll save you a lot of time in the long run and many modern IDE's offer refactoring tools. Keep in mind that full-caps member names (such as FRAMES_PER_SECOND) are used to indicate constants or defines. In another file, Player.h, you use m as prefix. Be consistent, either use the m_ and g_ or m and g prefixes everywhere.

Moving away from global variables is a good idea because they are often quick hacks but often they can do more bad than good in the long run. Imagine declaring g_GameTime as a global variable, and using that in every manager. But when we want a single manager to update at half speed to mimic slow motion, we'll have to do nasty things. So in my opinion it's better just to give the time as a parameter to every manager.

Defines

Header guards should be the first thing in a header file. In game.h this is not the case, so move those ifdef's up to save some compile time and potential compile errors.

Const correctness

If I call a function such as

bool Game::isRectCollision(sf::RectangleShape &sprite1, sf::Sprite &sprite2)

I want to be sure my parameters and base object will not be modified. (If calculating if two rectangles intersect requires non-const parameters (so possibly changing that rectangle), something fishy is going on). The general rule should be to mark everything as const unless it is inpossible:

bool Game::isRectCollision(const sf::RectangleShape &sprite1, const sf::Sprite &sprite2) const

Now, regarding your animation smoothness/stuttering problem. The following while loop is a bit weird:

while(timeSinceLastUpdate > TIME_PER_FRAME)

Since update is inside this loop and render is not, it is possible that we do not update and render the same state of the game again. The other way around: it is possible that we update the game twice but only render once. Both of these scenarios are wrong. The first one because we are going to render the exact same scene again, the second one because we're going to calculate how the scene should look like and then do nothing with the result, but rather calculate it again. For starters, try to replace this second while loop with an if and place the render functions inside of it.


MVC + Events in JavaScript I'm a bit lost with implementing MCV pattern on javascript.

Where should events (such as 'click','change') come in on MVC in javascript?

I have this event function,

   var EventExample = function (controller, view) {

       this.controller = controller;
       this.view = view;
       return this;
   };

   EventExample.prototype.clicked = function ( ) {

        var $root = this;

        var id = $("#input-text").val();

        $root.controller.find( id );

        return $root.view.render();
   };

And I have this function to bind all MVC, Mapper, Service, etc together,

   function bootstrapper() {

        // get a new model.
        var model = new ModelExample();

        // get a new model.
        var mapper = new MapperExample();

        // get a new service.
        var service = new ServiceExample( mapper, model);

        // get a new controller.
        var controller = new ControllerExample(service);

        // get a new view.
        var view = new ViewExample(model);

        // get a new event.
        var event = new EventExample(controller, view);

        $('#search-button').click(function(){
            event.clicked();
        });
   }

That is how I call the event function in there, but it seems weird.

I looked at this blog, the event (click) seems must be wrapped inside a function? if so, is it supposed to be in a controller, or in a model, or in a mapper?

Are all events considered Observers in the design patterns?

Any ideas how I should go with events?

My entire MVC code is on jsfiffle

Nowhere. Events have nothing to do with MVC.
Patterns are best practices for a specific solution
First of, read my rant on MVC: http://www.sitepoint.com/mvc-problem-solution/
MVC is a pattern that helps us in seperating our display, domain logic and data representation. Thats it. Nothing more.
It doesn't say anything about events or observers. Neither does it help us save the world.
MVC comunication
MVC states that a Controler updates a model, The Model updates the view and the view triggers the Controller.
It does however not say how this should happen. Should the Model be passed a view that it then updates. Maybe, maybe not. MVC doesn't care.
Because we are using JavaScript, we have however a very strong tool to solve this communication between the Model, view and controller; The event system.
Our Model-View communication could look like this:
model.on('update', function(updatedData) {
    view.update(updatedData);
});

This is called the Observer pattern. The anonymous function is the observer that observers the model. Here, the model notifies all registered observers of a change.
Or you could go for an event notifier. where you add notifiers and listeners:
controler.prototype.changeModelName = function() {
    myEventService.notify('model.update', {'data':{'name':'myNewName'}});
};
myEventService.listenTo('model.update', function(event){
    model.update(event.data);
});
myEventService.listenTo('model.update', function(event){
    view.update(event.data);
});

Or some other approach. MVC doesn't care.


Phone validation ecto model defmodule Baby.Post do
  use Baby.Web, :model

  schema "posts" do
    field :cover, :string
    field :email, :string
    field :firstname, :string
    field :lastname, :string
    field :birthday_day, :integer
    field :birthday_month, :integer
    field :birthday_year, :integer
    field :description, :string
    field :phone, :string

    timestamps
  end

  @required_fields ~w(email firstname lastname birthday_day birthday_month birthday_year description phone)
  @optional_fields ~w()

  @doc """
  Creates a changeset based on the `model` and `params`.

  If no params are provided, an invalid changeset is returned
  with no validation performed.
  """
  def changeset(model, params \\ :empty) do
    model
    |> cast(params, @required_fields, @optional_fields)
    |> validate_length(:description, min: 280)
    |> validate_format(:email, ~r/@/)
    |> valid_phone(:phone)
  end

  @doc """
  Check if it's a valid french number
  """
  def valid_phone(changeset, field) do
    phone = get_field(changeset, field)

    unless phone === nil do

      if String.length(phone) !== 10  do
        add_error(changeset, field, "n'est pas valide")
      else
        changeset
      end

    else
      changeset
    end

  end

end

I'm trying to add a custom validation for the phone field. The code is working, but it's really crappy. I'm quite new with elixir and I'm a bit confused about the fact the return is the last expression evaluated. Can you help me to rewrite that valid_phone method ?

When you end up with a pyramid of if-else clauses like this, it's sometimes a good idea to use cond instead.  cond lets you specify a bunch of clauses, and picks the first one that matches.

For example:

def valid_phone(changeset, field) do
  phone = get_field(changeset, field)
  cond do
    phone == nil ->
      changeset
    String.length(phone) != 10 ->
      add_error(changeset, field, "n'est pas valide")
    true ->
      changeset
  end
end

Of course, that can be simplified further into

def valid_phone(changeset, field) do
  phone = get_field(changeset, field)
  cond do
    phone != nil and String.length(phone) != 10 ->
      add_error(changeset, field, "n'est pas valide")
    true ->
      changeset
  end
end

Which can be simplified further, because any cond with one clauses & a true is really just a fancy if-else expression:

def valid_phone(changeset, field) do
  phone = get_field(changeset, field)
  if phone != nil && String.length(phone) != 10 do
    add_error(changeset, field, "n'est pas valide")
  else
    changeset
  end
end


Formatting list of queries in python I have a file of SQL (actually HiveQL) queries terminated by semicolon (;). I need to obtain a list of formatted queries from this file. Formatting includes replacing multiple spaces with single one and removing extra spaces in cases like '( ', ' )', ' ,'. So, the following is my approach and it looks monstrous:

fdHql = open(args.hql, 'r')
hql_file = fdHql.read()
fdHql.close()
queries = [re.sub('\s+', ' ', each.replace('\n', ' ').strip()).replace('( ', '(').replace(' )', ')').replace(' ,', ',') for each in hql_file.split(';')]

Is there a better way to implement same behavior?

You can eliminate the .close() using a with ... as ...: statement.

Since newlines are white-space characters, re.sub('\s+', ' ', ...) will already convert any '\n' characters to a space, so the .replace('\n', ' ') is unnecessary.

And you don't need to write everything on one line, even when you
want a "one-liner".

with open(args.hql, 'r') as fdHql:
    queries = [
               re.sub('\s+', ' ', each.strip())
                   .replace('( ', '(')
                   .replace(' )', ')')
                   .replace(' ,', ',')
               for each in fdHql.read().split(';')
              ]

Of course, the clearest is to move the work to a helper function.

def _format_sql(sql):
    return re.sub('\s+', ' ', sql.strip())      \
               .replace('( ', '(')              \
               .replace(' )', ')')              \
               .replace(' ,', ',')

with open(args.hql, 'r') as fdHql:
    queries = [ _format_sql(each) for each in fdHql.read().split(';') ]


Circular queue implementation I've implemented a simple thread-safe circular queue. I'm looking for code review, optimizations and best practices.

public class CircularQueue {
    int[] queue;
    int front;
    int rear;
    int currentSize;
    int size;

    public CircularQueue(int size) {
        this.queue = new int[size];
        this.front = 0;
        this.rear = 0;
        this.size = size;
        this.currentSize = 0;
    }

    public synchronized boolean add(int x) {

        if (currentSize == size) {
            throw new IllegalStateException("The queue is full: front size: " + rear);
        }

        queue[rear++] = x;
        rear = (rear + 1) % size;
        currentSize++;

        return true;
    }

    public synchronized int peek() {
        if (front == rear) {
            throw new IllegalStateException("The queue is empty");
        }
        return queue[front];
    }

    public synchronized int poll() {
        if (front > rear) {
            throw new IllegalStateException("The queue is empty");
        }
        currentSize--;
        return queue[front++];
    }
}

Explicit visibility

I don't like relying on standard visibility; explicit declaring takes away all doubts. Instead have this:

private int[] queue;
private int front;
private int rear;
private int currentSize;
private int size;

Implicit this

Contrary to the above having explicit this is unneeded and takes away from the readability. You should only use it when there is ambiguity with a parameter name. This results in this:

queue = new int[size];
front = 0;
rear = 0;
this.size = size;
currentSize = 0;

Naming

What is currentSize and how does it differ from size? At first glance I would think they're the same. Some alternative names

size:

size
maxSize

currentSize:

index
offset

Iterator

If you want your class to be able to be used in a foreach loop, you could implement Iterator<T>.

Generics

Your datastructure is not generic so you're losing out on a lot of possibilities.

Kaput

Your code isn't working as it is. I'm not entirely sure why (it's 3:30 AM so I'm not going to debug it deeply anymore) but I followed a hunch and this small example demonstrates it:

public static void main(String[] args) {
    CircularQueue cq = new CircularQueue(3);
    cq.add(5);
    cq.add(10);
    System.out.println(cq.poll());
    cq.add(15);
    System.out.println(cq.poll());
}

You'll notice that the last call to poll() will throw you your self defined exception (aka front > rear evaluates to true).


Subtraction accumulator I'm currently following the tutorials over at RubyMonk, and one of the problems I need to solve is to write a subtract function that would meet these conditions:

  invoking subtract(4, 5) should return -1
  invoking subtract(-10, 2, 3) should return -15
  invoking subtract(0, 0, 0, 0, -10) should return 10

Coming from a traditional imperative programming background (C, Lua, Java, etc.), my first attempt would have been something like this:

def subtract *numbers
  start = numbers[0]
  tail  = numbers.drop(1)

  for i in tail do
    start -= i
  end

  return start
end

But, this just felt wrong in Ruby, and I don't doubt that it is. Trying to use a more Ruby-esque style, I thought it would be better to use some of Array's methods.

Here is my latest version:

def subtract *numbers
  (numbers.drop 1).inject(numbers[0]) { |x, y| x-y}
end

One thing to note is that this is essentially the same thing as above, I just moved most of the looping into Array#inject. To me, simply moving logic isn't a new style per se, and so conceptually this is nothing new to me.

Therefore my questions are:

Is this true Ruby style? If not, how could I make it so, and how would that new style differ from the current?
Is there any way to improve readability? ('Cause honestly the current version lacks quite a bit).

Some notes:

def subtract *numbers: While Ruby allows to omit parens, the community consensus is that it makes signatures harder to read.
(numbers.drop 1): When you have to write parens like this, it's a signal that you should just write them in the method call. It looks like Scala or Haskell, not Ruby.
As @Nakilon pointed out, you can drop the drop/first, a fold without an initial value takes the first one.
The identity value for subtraction is 0, let's use it for empty inputs.
xs.inject(initial) { |acc, x| acc.method(x) } -> xs.inject(initial, :method)

We can now write:

def subtract(*numbers)
  numbers.inject(0, :-)
end

  For me, simply moving logic isn't a new style per se, and so conceptually this is nothing new to me.

I don't agree. The steps followed are conceptually the same, yes, but the use of a widely known generic abstraction like reduce/inject has reduced the overall complexity. Of course, one abstraction does not make much difference, but dozens of them change the way a whole program describes what it's doing. And that's what programing is about: building abstractions to reduce complexity.


Places and their PlaceCategories I'm going to start small here because I actually have several questions but I'll go one at a time.

In the app I am making to better learn Java/Android/SQL/OOP, I have a page where a user can define Places (e.g. "New York City", "Seattle", etc). You could add made-up stuff if you wanted, too, doesn't matter. The user can perform basic CRUD operations on these Places.

Each Place also has a PlaceCategory, which is also user-defined elsewhere in the app in a separate list. So maybe I associate NYC and Boston with "The Northeast" and so if I click the filter dropdown and select "The Northeast," then NYC and Boston remain and the rest disappear (until I remove the filter again).

This is my SQL for the PlaceCategory table:

private static final String STRING_CREATE_TABLE_PLACE_CATEGORIES =
        "CREATE TABLE PLACE_CATEGORIES ("
                + "ID INTEGER PRIMARY KEY AUTOINCREMENT, "
                + "CATEGORY_NAME TEXT UNIQUE NOT NULL)";

And for the object:

public class PlaceCategory implements Comparable<PlaceCategory> {
    private long mId;
    private String mName;

    public PlaceCategory() {
        mId = -1;
        mName = "";
    }

    public PlaceCategory(long id, String name) {
        mId = id;
        mName = name;
    }

    public long getId() {
        return mId;
    }

    public void setId(long id) {
        mId = id;
    }

    public String getName() {
        return mName;
    }

    public void setName(String name) {
        mName = name;
    }

    @Override
    public int compareTo(PlaceCategory another) {
        return mName.compareTo(another.getName());
    }
}

And now for Places:

private static final String STRING_CREATE_TABLE_PLACES =
        "CREATE TABLE PLACES ("
                + "ID INTEGER PRIMARY KEY AUTOINCREMENT, "
                + "PLACE_CATEGORY_ID INTEGER NOT NULL, "
                + "PLACE_NAME TEXT NOT NULL, "
                + "PLACE_DESC TEXT NOT NULL, "
                + "FOREIGN KEY("
                    + "PLACE_CATEGORY_ID) REFERENCES "
                    + "PLACE_CATEGORIES(ID) ON DELETE CASCADE, "
                + "UNIQUE("
                    + "PLACE_CATEGORY_ID,"
                    + "PLACE_NAME))";

And the object:

public class Place implements Comparable<Place>{
    private long mId;
    private long mCategoryId;
    private String mName;
    private String mDesc;

    public Place() {
        mId = -1;
        mCategoryId = -1;
        mName = "";
        mDesc = "";
    }

    public Place(long id, long categoryId, String name, String desc) {
        mId = id;
        mCategoryId = categoryId;
        mName = name;
        mDesc = desc;
    }

    public long getId() {
        return mId;
    }

    public void setId(long id) {
        mId = id;
    }

    public long getCategoryId() {
        return mCategoryId;
    }

    public void setCategoryId(long categoryId) {
        mCategoryId = categoryId;
    }

    public String getName() {
        return mName;
    }

    public void setName(String name) {
        mName = name;
    }

    public String getDesc() {
        return mDesc;
    }

    public void setDesc(String desc) {
        mDesc = desc;
    }

    @Override
    public int compareTo(Place another) {
        return mName.compareTo(another.getName());
    }
}

My questions:

Should I be using long or Long for the ids in general? I used long because I don't think I need the null-ability of Long but I am unsure what the accepted practice is here.
Should I be storing the Category as an object in the Place, as opposed to an id like I am currently doing? My concern with storing only the id is that I would have to query the database for the category details whenever I wanted more detail. I don't know if this is considered a good idea. On the other hand, it is easier to query/generate Places when their members are primitive datatypes that can be ripped straight out of the table.
Are the ways my tables set up normalized properly? Do they make sense?
Anything else that stands out?

#3 Normalization

What will you do if a place belongs to more than one category?  For example, I might categorize Boston as New England as well as The Northeast.  But you only have one place category ID per place.

If the category is always a region, you might call the table Regions instead.  That's more specific than category or even place category.

I would expect something called place categories to know to what place it belonged.  But in this case, one place category has many places.  This is part of why I don't like the name.  It feels wrong to me.

#2 Object vs. ID

Hibernate is an Object-relational Mapping tool (ORM).  It always includes related tables as objects, but it doesn't necessarily bother loading them unless needed.  I.e. it has a lazy loading option.  It's application specific which way you do it.  If you normally need it, perhaps load it aggressively.  If you normally don't need it, perhaps you only load it lazily.

Note that place categories are rather light weight.  It would be easy enough to load all of them and assign them as you load places.  That's a good pattern if you have to load all the categories in order to make your drop down.


MVC Improvement - The View Module - 1 /****************************************************************
VIEW MODULE
view, view_database, view_message
view_arche_1 (external), view_arche_2 (external) - these are
  primarily html files
****************************************************************/

/*view*/

class view extends database
  {
  }

/*view_db*/

class view_db extends view
  {
  function __construct($type)
    {
    parent::__construct();
    $this->invoke($type);
    }
  private function invoke($type)
    {
    switch ($type)
      {
      case "bookmarks":
        $this->html_bookmarks();
        break;
      case "tweets":
        $this->html_tweets();
        break;
      default:
        throw new Exception('Invalid View Type');
        break;
      }
    }
  private function html_bookmarks()
    {
    $email = $_SESSION['email'];
    $query_return = database::query("SELECT * FROM bo  WHERE email='$email' ORDER BY name ASC");
    $html_string='';
    while ($ass_array = mysqli_fetch_assoc($query_return))
      {
      $fav=$this->favicon($ass_array['url']);
      $html_string = $html_string . "$ass_array[name]";
      }
    echo $html_string;
    }
  private function html_tweets()
    {
    $query_return = database::query("SELECT * FROM tw ORDER BY time DESC LIMIT 7");
    $time = time();
    $html_string='';
    while ($a = mysqli_fetch_assoc($query_return))
      {
      $html_string = $html_string . "
$a[fname] posted

$a[message]

";
      }
    echo $html_string;
    }
  private function favicon($url)
    {
    $pieces = parse_url($url);
    $domain = isset($pieces['host']) ? $pieces['host'] : '';
    if(preg_match('/(?P<domain>[a-z0-9][a-z0-9\-]{1,63}\.[a-z\.]{2,6})$/i', $domain, $regs))
      {
      return $pieces['scheme'] . '://www.' . $regs['domain'] . '/favicon.ico';
      }
    return false;
    }
  }

class view_message extends view
  {
  function __construct($type)
    {
    $this->invoke($type);
    }
  private function invoke($type)
    {
    $this->message($type);
    }
  private function message($type)
    {
    $type_message = array(
    'empty' => '<si_f>Please complete all fields.',
    'name'=> '<su_f>Only letters or dashes for the name field.',
    'email' => '<si_f>Please enter a valid email.',
    'taken' => '<si_f>Sorry that email is taken.',
    'pass' => '<si_f>Please enter a valid password, 6-40 characters.',
    'validate' => '<si_f>Please contact support to reset your password.');
    echo $type_message[$type];
    }
  }

Some random observations:

What does your base class database do? I hope not the stuff it sounds like. Your aim is to separate storage and view logic, not add view logic to your database.
Having both static and non-static functions in a class should make you suspicious (as this is an indicator this class has one then more objective or is inconsistently written).
Some functions do echo, some return and some call other functions. Decide for one way ;).
invoke for parameter to function resolving? Have a look at the much cooler magic function __call.

Some suggestions:

It is probably a good idea to move the single view operations into it's own classes. This becomes more important when you add even more operations.
Unify the way your class is accessed and the return values.
Views don't extend a database.

Easy to implement way:

Let every view operation be it's own class.
Define a common interface for all view operations (e.g. ViewOperation with one required function run()).
Create one main class View that is responsible to resolve view operations to subclasses.

Example implementation:

interface ViewOperation
{
    public function run();
}

public class ControllAdd implements ViewOperation
{
    private $_types = array('pass' => '<xx_p>', 'fail' => '<xx_f>');

    public function run()
    {
        $params = func_get_arg(1);
        if(isset($params) && !empty($params) && array_key_exists($this->_types, $params))
        {
            return $this->_types[$params];
        }
    }
}

public class View
{
    public function __call($name, $arguments)
    {
        // More security required here of course
        require_once './ViewOperartopns/' . $name . '.php';

        $instance = new $name;
        return $instance->run($arguments);
    }
}


Lazy generator of strings in lexicographical order I needed to write a Python generator that lazily generates, in lexicographical order, strings composed of a certain alphabet (e.g. lowercase English letters).
My first thought was to make an infinite generator, but an infinite iterator would be pretty useless because it would just generate strings composed of only the character a. This is because the lexicographical ordering of strings is not a well-order; it can be thought of as composed of an infinite sequence of infinitely nested sequences: (a, (aa, ...), (ab, ...), ...), (b, (ba, ...), (bb, ...), ...), ... The generator would never reach ab since it has an infinite amount of predecessors.
Thus I went for limiting the maximum length of the strings. I.e., the full specification is a generator that lazily generates all strings in a given alphabet of up to a certain length in lexicographical order. E.g., for a maximum length of 3 with ASCII lowercase letters, it should generate, in this order: "", "a", "aa", "aaa", "aab", ..., "aaz", "ab", "aba", ..., "zzz".
The solution I came up with is elegant in the abstract; it uses the intrinsic recursive relationship of lexicographical ordering: if you take the set of all strings—including infinite strings—and remove the first character from each, you're still left with the set of all strings. But it's probably not very efficient in practice, as for each iteration it uses Python's string concatenation operation + \$m\$ times to build a string of length \$m\$ (\$m - 1\$ times in between characters plus the "terminating" empty string, as you'll see below), and strings are immutable so copies are made at every intermediate step, resulting in \$O(m^2)\$ time complexity at each iteration.
Here is my solution:
import string

def lexstrings(max_length: int, alphabet=string.ascii_lowercase):
    yield ""
    if max_length == 0: return
    for first in alphabet:
        for suffix in lexstrings(max_length - 1, alphabet=alphabet):
            yield first + suffix

Example:
>>> g = lexstrings(max_length=3, alphabet="ab")
>>> list(g)
['',
 'a',
 'aa',
 'aaa',
 'aab',
 'ab',
 'aba',
 'abb',
 'b',
 'ba',
 'baa',
 'bab',
 'bb',
 'bba',
 'bbb']

This implementation also "supports" the infinite version by supplying a negative maximum length:
>>> g = lexstrings(-1)
>>> next(g)
''
>>> next(g)
'a'
>>> next(g)
'aa'
>>> next(g)
'aaa'
...

Some benchmarking confirms a single iteration of this is quadratic in terms of the string size. Here is a plot of execution time for generating a string of maximum length (in a single iteration) vs the maximum length:

The code to generate this plot using perfplot follows. (I modified the order of generation so that the first iteration yields a max-length string.)
import string
import perfplot
import sys

import numpy as np

def lexstrings(max_length: int, alphabet=string.ascii_lowercase):
    if max_length == 0:
        return
    for first in alphabet:
        for suffix in lexstrings(max_length - 1, alphabet=alphabet):
            yield first + suffix
    yield ""

n_range = np.linspace(1, sys.getrecursionlimit() - 500)
benchmark = perfplot.bench(
    setup=lambda n: n,
    kernels=[
        lambda n: next(lexstrings(n)),
    ],
    labels=[
        "lexstrings",
    ],
    n_range=n_range,
    xlabel="maximum string length",
    title="Single iteration (max-length string)",
    target_time_per_measurement=0.1,
    equality_check=None,
)

benchmark.show(logx=False, logy=False)

It's a great question, and the only improvements I can see are superficial ones:

Your method has incomplete type hints. alphabet should be Iterable[str], and it should return -> Iterable[str] as well.
PEP8 discourages multi-statement lines (your return).

Otherwise, you should reconsider making this a recursive implementation. I see that you were careful to check sys.getrecursionlimit() in your test code, so you're already aware of the fact that this will break given sufficient depth. Python also does not have tail optimization so an iterative implementation will potentially yield performance improvements.


Decimal to binary array Basically, this program takes a decimal number from 1 to 10, that is the index of power of 2. And turns into an array with \$2^n\$ combinations.

For example: If my decimal is 1, I'll have 2 arrays, [0] and [1], and if my decimal is 2, I'll have 4 combinations, [0 0]; [0 1]; [1 0]; [1 1], and so on. I cannot use break conditions.

#include <stdio.h>

int main() {
    int n, q = 1, i, f, d, p, vet[10]; // n: index; q: combinations; i, f, d, p: ints for operations;

    scanf ("%d", &n); //index

    for (i = 0; i < n; i++){ // getting total number of comb
        q *= 2;
    }
     n--;

    for (i = 0; i < q; i++){
        f = i;

        for (p = 0; p < 10; p++){   //setting array to 0
            vet[p] = 0;
        }

        while ( f > 0){
           p = 1;
           d = 0;

            while (f >= 2*p){
                p *= 2;
                d++;
            }

            vet[d] = 1; //changing position 'd' on array for 1.
            f -= p;
        }

        // printing results
        printf ("f: %d bin:", i);

        for (p = 0; p < 10; p++)
            printf ("%d", vet[9-p]);

        printf ("\n");
    }
    return 0;
}

Some comments:

There is no need to store the results in memory, so don't. Simply output them as they are computed.
It's been many years since C has allowed variables to be declared where they're used instead of being lumped together at the top. Do this for legibility.
Don't loop through to get q. Powers of 2 are easily found via << .
Don't loop through to reset the array contents to 0. Simply set the elements to their desired value.
Don't populate an array forward and then loop back through it in reverse to do an output. Simply populate a string that is ready to be output.
f doesn't mean anything to the program user, so don't use it in the output; use something like "decimal" instead.

An alternative implementation is:

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

int main()
{
    int n;
    scanf("%d", &n);

    char *output = malloc(n+1);
    output[n] = '\0';

    for (int number = 0; number < (1 << n); number++)
    {
        int temp_num = number;
        for (int idx = n-1; idx >= 0; idx--)
        {
            output[idx] = '0' + (temp_num & 1);
            temp_num >>= 1;
        }
        printf("dec: %d bin: %s\n", number, output);
    }
    return 0;
}


Converting a String to USD I'm working on a clientside iOS app for my school that involves students purchasing things, and I wrote a String extension for formatting a String into USD. Is there a way to improve this or make it more "swifty"?

extension String {
    var usdFormat: String {
        var str = self
        var postDec = str.components(separatedBy: ".")
        switch postDec[1].count {
        case 0: str.append("00")
        case 1: str.append("0")
        default: break
        }

        str = "$" + str

        return str
    }
}

... for formatting a String into USD ...

A string is not the appropriate type to hold a monetary amount. What would your
extension method return if called on the string "abc" or "?" ? A price is a number
and should be stored as such. Possible choices are

An integer holding the price in units of the smallest denomination (e.g. cents), or
A Decimal which represents a base-10 number with up do 38 decimal digits.

Creating a textual representation of a number is the job for a
NumberFormatter.
It has options to specify the desired number of fractional digits, and even
has a dedicated .currency
style.

Using a number formatter has several advantages over “manually” formatting the price,
such as:

It chooses the appropriate style according to the user's locale.
It puts the currency symbol at the correct position.
It rounds the number to the correct number of decimal places.

Formatting a US dollar currency value is as simple as

let formatter = NumberFormatter()
formatter.numberStyle = .currency
formatter.currencyCode = "USD"

let usPrice: Decimal = 1234.56
let displayString = formatter.string(for: usPrice)!
print(displayString)

This produces the string $1,234.56 if the user's locale is "en_US".
On my device (with a german locale) it produces 1.234,56 $.


VB6/VBA Declaration (Dim) Syntax Parser As part of a project I'm working on, I've implemented a class that encapsulates the syntax for a vb6/vba Dim statement.
Given a line of code, the Parse method uses a regular expression to match the keyword's syntax and extract the relevant parts.
As it stands there's no support for some valid syntaxes, the following aren't currently supported:

'multiple declarations on the same line:
Dim foo As String, bar As String

'multiple instructions on the same line:
Dim foo As String: Dim bar As String

But this question isn't about the code that's not yet implemented, it's about the approach taken; the regular expression works (for the so-far supported use cases), but it's quickly going to become a problem (read: mess), especially since DimSyntax isn't going to be the only ISyntax implementation in the code.
On the other hand, vb6 isn't likely to ever change in the future, so perhaps a well-crafted regex can do the job, if it won't need to be maintained?
public class DimSyntax : ISyntax
{
    // todo: ditch regex
    private static readonly string _syntaxPattern =
        @"^((?<keyword>Dim)\s(?<name>\w{1,30})\s?(As\s(?<type>\w{1,30})))?";

    private static readonly Regex _regex = new Regex(_syntaxPattern);

    private readonly IVBTypeFactory _typeFactory;

    public DimSyntax(IVBTypeFactory typeFactory, IIdentifier identifier)
    {
        _typeFactory = typeFactory;
        _identifier = identifier;
    }

    private readonly IIdentifier _identifier;
    public IIdentifier Identifier { get { return _identifier; } }

    public string Keyword { get { return Keywords.Dim; } }

    public ISyntax Parse(string line)
    {
        var code = line.Trim();

        var match = _regex.Match(code);
        if (!match.Success)
        {
            return null;
        }

        var nameGroup = match.Groups["name"];
        var typeGroup = match.Groups["type"];
        if (nameGroup == null || typeGroup == null)
        {
            return null;
        }

        var type = _typeFactory.Parse(nameGroup.Value);
        var identifier = new Identifier(nameGroup.Value, type);

        return new DimSyntax(_typeFactory, identifier);
    }

    public override string ToString()
    {
        if (_identifier.Type != null)
        {
            // explicit type
            return string.Format("{0} {1} {2} {3}", Keyword, _identifier.Name,
                                                  Keywords.As, _identifier.Type.TypeName);
        }
        else
        {
            // implicit type - Variant
            return string.Format("{0} {1}", Keyword, _identifier.Name);
        }
    }
}

Keywords is just a .resx resource file that contains a list of vb6/vba keywords. I was going to make a public static class with a bunch of public static readonly string members, but I find it's cleaner to use a .resx file. Other suggestions?
I have mixed feelings about the IVBTypeFactory being a dependency of the type, when it's really a dependency of the Parse method - would it make sense to just take the abstract factory in as a method parameter instead?

For reference & context, here's the Identifier implementation:
public class Identifier : IIdentifier
{
    public Identifier(string name, IVBType type)
    {
        _name = name;
        _type = type;
    }

    private readonly string _name;
    public string Name { get { return _name; } }

    private readonly IVBType _type;
    public IVBType Type { get { return _type; } }
}

Pretty much all types are immutable, which means any number of threads can safely access the parsed ISyntax objects - am I making my life harder than it needs to be here?
If you need more context, or are curious to see more, the rest of the code can be found on GitHub.

Finally, a question where we can use Roslyn!

I won't review your own code but instead I whipped up this small showcase of how you might do it with the Roslyn compilers. Do note that it is rather ugly, not in the least because I am not entirely comfortable with it myself yet but also because I know little VB.NET and it appears some things are handled differently from the C# compiler. That being said, this sample implementation can parse both

Dim foo As String

and

Dim foo As String, bar As String

El codos:

using System;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.VisualBasic.Syntax;
using Microsoft.CodeAnalysis.VisualBasic;

class Program
{
    static void Main(string[] args)
    {
        var stuffIWantToParse = "Dim foo As String, bar As String";

        var tree = VisualBasicSyntaxTree.ParseText(@"
            Module Module1
                 " + stuffIWantToParse +
            @" End Module
        ");

        var compilation = VisualBasicCompilation.Create("MyCompilation", syntaxTrees: new[] { tree });
        var moduleType = compilation.GetTypeByMetadataName("Module1");

        PrintVariableDeclarationByIndex(0, moduleType);
        PrintVariableDeclarationByIndex(1, moduleType);

        Console.Read();
    }

    private static void PrintVariableDeclarationByIndex(int i, INamedTypeSymbol moduleType)
    {
        var variable = moduleType.GetMembers().Skip(i).FirstOrDefault();

        var position = variable.Locations.First().SourceSpan.Start;
        var root = variable.Locations.First().SourceTree.GetRoot();
        var parent = root.FindToken(position).Parent;
        var node = parent.FirstAncestorOrSelf<FieldDeclarationSyntax>();
        var typeInfo = node.Declarators[i].AsClause;

        Console.WriteLine("Modifiers: " + string.Join(",", node.Modifiers));
        Console.WriteLine("Name: " + variable.Name);
        Console.WriteLine("Type: " + typeInfo.Type());
        Console.WriteLine();
    }
}

Output:

Using a workaround from here. If you want some more codesamples on Roslyn, look at my project here. In order to use the Visual Basic compilers look here. For more information on Roslyn, take a look at the roslyn tag wiki I put together.


MyTimer based on QTimer I needed a timer that fires in intervals for a given duration when e.g. a button is pressed. The button can be pressed several times thus I thought it would be easiest to create a new timer for each button press and the timer destroys itself when its duration passed. I wrote this...

Interface:

// timerhandler.h
class TimerHandler {
    public:
        virtual void processTimer()=0; // callback
        void startNewTimer(int interval,int duration);
        virtual ~TimerHandler(){}
};

//timerhandler.cpp
#include "timerhandler.h"
#include "mytimer.h"
void TimerHandler::startNewTimer(int interval,int duration){
    new MyTimer(this,interval,duration);
}

Usage: MainWindow inherits TimerHandler

//MainWindow.cpp
void MainWindow::on_pushButton_start_clicked() { this->startNewTimer(100,2000); }
void MainWindow::processTimer() { std::cout << "TIMER" << std::endl; }

Implementation:

//mytimer.h
#include <QObject>
#include <QTimer>
class TimerHandler;
class MyTimer : public QObject {
        Q_OBJECT
    public:
        friend class TimerHandler;
    private:
        TimerHandler* th;
        QTimer* timer;
        MyTimer(){};
        MyTimer(TimerHandler* th,int interval,int duration);
        ~MyTimer();
    public slots:
        void commitSuicide();
        void on_timer_fired();
};

//mytimer.cpp
MyTimer::MyTimer(TimerHandler* th,int interval,int duration) : th(th) {
    timer = new QTimer(this);
    connect(timer, SIGNAL(timeout()), this, SLOT(on_timer_fired()));
    timer->start(interval);
    QTimer::singleShot(duration,this,SLOT(commitSuicide()));
}
MyTimer::~MyTimer(){ delete timer; }
void MyTimer::commitSuicide(){ delete this; }
void MyTimer::on_timer_fired(){ th->processTimer(); }

...it works, but I wonder if there is an easier way (less code) and it is horribly inflexible. I am a bit stuck when I want to change the code to allow different callbacks (that take different parameters).

I first tried a virtual slot as interface. I think it would work somehow, but when TimerHandler inherits QOject, then my MainWindow inherits QObject twice and I could not get it running.

Also if there is anything else that I can/should fix please let me know.

That's a lot of code indeed, and for all that code it is not very flexible: the "slot" name is fixed, you force multiple inheritance on your users, so they can have only one such type of timer - what if I want one to call processThing() and another to do processOtherThing?

You should use the signal/slot mechanism to your advantage - that's what it is there for, and get rid of your interface and class entirely: a single (global/static) function is enough.

Consider something like this (QTimer::singleShot itself is a good example of how you should be doing it):

void limited_timer(int interval, int duration, QObject *target, const char *slot)
{
  QTimer *timer = new QTimer;
  QObject::connect(timer, SIGNAL(timeout()), target, slot);
  QTimer::singleShot(duration, timer, SLOT(deleteLater()));
  timer->start(interval);
}

Usage (in one of your main window's methods):

limited_timer(100, 2000, this, SLOT(processTimer()));


Returning the path to PHP pages for a layout and a view, if they exist I have these two functions.

function layout($layout) {
    $layout = Config::get('path/layouts') . '/' . $layout . '.layout.php';
    if (!file_exists($layout)) {
        throw new CoreException($layout . ' layout is not found');
    }
    return $layout;
}

function page($page) {
    $page = Config::get('path/pages') . '/' . $page. '.view.php';
    if (!file_exists($page)) {
        throw new CoreException($page . ' page is not found');
    }
    return $page;
}

Is there a way to avoid repeated parts?

Yes, there is a way:

Identify all parts that differ in the two functions.
Write a new function that takes each of these differences as a parameter.
From the existing functions, call the new function, passing the appropriate values as parameters.
Compare the resulting code with the original code to see whether the refactoring was worth it.

Hint:

   In this case, the differing parts are configKey, fileName, suffix and errorMessage.


Returning a different but similar "increasing" string depending on instance argument I have to work with a function that takes an instance of a class as an input, and depending on that instance, I have to return a specific string. It works, but it has a lot of repetitive code. How can I make it more efficient?

private getDescription(ScreenViewer o) {
    String output = "";
    if(Utils.isTotalPagesVisible()) {
       String total = 5;
       if(o instance of ScreenViewerPage1) {
           output = " 1 out of " + total;
       }
       if(o instance of ScreenViewerPage2) {
           output = " 2 out of " + total;
       }
       if(o instance of ScreenViewerPage3) {
           output = " 3 out of " + total;
       }
       if(o instance of ScreenViewerPage4) {
           output = " 4 out of " + total;
       }
       if(o instance of ScreenViewerPage5) {
           output = " 5 out of " + total;
       }
    }
    else {
        String total = 3;
       if(o instance of ScreenViewerPage1) {
           output = " 1 out of " + total;
       }
       if(o instance of ScreenViewerPage2) {
           output = " 2 out of " + total;
       }
       if(o instance of ScreenViewerPage3) {
           output = " 3 out of " + total;
       }
    }
   return "Description:" + output;
}

The easiest way is to add a method to the ScreenViewer class which returns the page value, eg:

public abstract class ScreenViewer {
    public abstract int getPageCount();
}

public class ScreenViewerPage1 {
    @Override
    public getPageCount() {
        return 1;
    }
}

private getDescription(ScreenViewer o) {
    if(Utils.isTotalPagesVisible()) {
        int total = 5;
    } else {
        int total = 3;
    }
    return "Description:" + o.getPageCount() + " out of " + total ;
}

But I would rethink your whole approach. What if you want to add ScreenViewerPage6? It's hard to say how exactly your code could be rewritten without knowing how the classes look, but I would assume that it should be possible with lists, etc.

Misc

Your indentation is off.
o is not a good variable name.
use else if when appropriate.
I would not hardcode magic numbers such as 5 and 3.


Comparing algorithms to decode a String I am writing a function to decode a string, which means performing a string transformation like 3e4f2e --> eeeffffee.

I have two versions of codes which are very similar but slightly different. In version1,

def decoding(s):
    res = []
    curr = 0
    curr_val = 0
    while curr < len(s):
        if not s[curr].isdigit():
            res.append(curr_val * s[curr])
            curr_val = 0
        else:
            curr_val = curr_val * 10 + int(s[curr])
        curr += 1
    return ''.join(res)

I keep in track of curr_val and keep accumulating the value until I see a non-digit string.

In version2, I keep in track of the first position of the digit string and slice the string just to represent the digit string.

def decoding(s):
    digit_start, res = 0, []
    curr = 0
    while curr < len(s):
        if not s[curr].isdigit():
            res.append(int(s[digit_start:curr]) * s[curr])
            digit_start = curr + 1
        curr += 1
    return ''.join(res)

I just keep in track of the right index and multiply the int of that string with the current non-digit string (e.g. 10a --> aaaaaaaaaa)

I wonder if the first version has a way better time complexity or if they have the same time complexity in big-O. I assumed that both of them are \$O(n)\$ where \$n\$ is the length of the input string, but I wonder if slicing inside a loop would significantly increase the time-complexity of the code.

Please note that this is an interview practice, so I care about time-complexity, not the real-world style.

They both look like they are \$O(n)\$.

A quick check with timeit show they both take essentially the same amount of time, and the time grows linearly with the length of the input string.

It would be better to iterate over the characters in the string rather use an explicit index over the length of the string (while curr < len(s) is often a smell).

def decoding(s):
    result = []
    repeat = 0

    for c in s:
        if c.isdigit():
            repeat = 10 * repeat + int(c)

        else:
            result.append(repeat * c)
            repeat = 0

    return ''.join(result)

or:

def decoding(s):
    result = []
    prev_index = 0

    for index,c in enumerate(s):
        if not c.isdigit():
            result.append(int(s[prev_index:index]) * c)
            prev_index = index + 1

    return ''.join(result)


Can someone have a look to my Chess Project? I am currently working on an unfinished project about Chess, it will (at some point) be able to help me to make some chess variations just by pasting the FEN position while I play online.

But before continuing, my code must have good structure or it will become a nightmare to fix it later. So now that I have some basics, it would be great to have it reviewed:

//Note: the code uses jQuery

function writeBoard() {
    var i, j, str, bol;

    str = "

";
    for (i = 9; --i;) {
        str += "
";
        for (j = 0; j < 8; j++) {
            bol = !bol;
            str += "	";
        }
        bol = !bol;
        str += "
";
    }
    str += "

";

    $("body").append(str);
}

function setFEN(fen) {
    var i, j, len, arr, tmp, tmp2, tmp3;

    arr = (fen ? fen : DefaultFen).split(" ")[0].split("/");
    for (i = 0; i < 8; i++) {
        tmp = i * 8;
        for (j = 0, len = arr[i].length; j < len; j++) {
            tmp2 = arr[i].charAt(j);
            if (tmp2 * 1) {
                tmp += tmp2 * 1;
            } else {
                tmp3 = tmp2.toLowerCase();
                ChessBoard[i][tmp % 8] = PiecesNames.indexOf(tmp3) * (tmp2 != tmp3 ? 1 : -1);
                tmp++;
            }
        }
    }

    refreshBoard();
}

function refreshBoard() {
    var i, j, emt, fen, tmp, tmp2, tmp3, tmp4;

    fen = "";
    for (i = 9; --i;) {
        emt = 0;
        for (j = 0; j < 8; j++) {
            tmp = Math.abs(i - 8);
            tmp2 = ChessBoard[tmp][j] || 0;
            if (tmp2) {
                if (emt) {
                    fen += "" + emt;
                    emt = 0;
                }

                tmp3 = $("#" + AbcLabels[j] + "" + i);
                if (tmp2 < 0) {
                    tmp4 = PiecesNames.charAt(tmp2 * -1);
                    tmp3.addClass("b" + tmp4);
                    fen += tmp4;
                } else {
                    tmp4 = PiecesNames.charAt(tmp2);
                    tmp3.addClass("w" + tmp4);
                    fen += tmp4.toUpperCase();
                }
            } else {
                emt++;
            }
        }

        if (emt) {
            fen += "" + emt;
        }
        fen += "/";
    }

    console.log(fen); //remove the last "/"
    //fen+= can enPass, to move, castle Avility, clock, etc...
    //Fen=fen
}

var I, J, Len;
var AbcLabels = ["a", "b", "c", "d", "e", "f", "g", "h"];
var PiecesNames = "*pnbrqk"; //[wp:1][wn:2][wb:3][wr:4][wq:5][wk:6]***[bp:-1][bn:-2][bb:-3][br:-4][bq:-5][bk:-6]
var DefaultFen = "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq -";
var ChessBoard = new Array(8);
for (I = 8; I--;) {
    ChessBoard[I] = new Array(8);
}
var Fen;
var ToMove; //[0:white][1:black]
var WCastling, BCastling; //[0:none][1:short][2:long][3:both]
//var IsCheck,EnPassant;

$(function() {
    writeBoard();
    setFEN("5Rk1/1b2p2p/p2pP1p1/2rP4/2BQ2P1/6qP/PP6/1K6 b - -");
});

If you don't have a good text editor, you will find tmpX variables very hard to recognize one over the another... In my case, I just double click and they all get highlighted (using Notepad++).

The first thing I noticed are the escaping orgies. Both HTML and JavaScript allow you to use either ' or " to quote strings. Don't use the same for both and you can get rid of all those ugly backslashes.

You can also get rid of the (badly-named) bol variable. You already have a loop variable so you can use j % 2 == 0 or even simpler j & 1 to determine if the number is even or odd.


Sentinel linear search I'm pretty new to algorithms, and it's my first attempt to implement sentinel linear search in JS. I'm wondering if it can be improved.
const array1 = ['lorem', 'ipsum', 'dolor', 'sit', 'amet', 'consectetur', 'adipiscing', 'elit', 'cras', 'eu', 'metus', 'volutpat', 'sagittis', 'nunc', 'in', 'porta', 'quam'];
let i = 0;
let x = 'elit';
let n = array1.length;
let last = array1[n - 1];
array1[n - 1] = x;

(function () {
    while (array1[i] !== x) {
        i++;
        if (array1[i] === x) {
            array1[n - 1] = last;
            if (array1[i] === x) {
                return console.log('FOUND ON INDEX', i);
            } else {
                return console.log('NOT FOUND');
            }
        }
    }
})();

If the point of the sentinel is to, as the wiki article says:

The basic algorithm above makes two comparisons per iteration: one to check if Li equals T, and the other to check if i still points to a valid index of the list. By adding an extra record Ln to the list (a sentinel value) that equals the target, the second comparison can be eliminated until the end of the search, making the algorithm faster.

Then the approach should be to tack on the value to find onto the end of the array, instead of replacing the last value of the array with the value to find. I guess you can replace the last value, then once a match is found, replace it back, then check it, but that's unnecessarily convoluted.
Your code has a bug: if the first element of the array is the item to search for, while (array1[i] !== x) { will evaluate to false, and the IIFE will terminate immediately without logging anything.
That test is superfluous anyway, since inside the loop, the array item will either be found before the end, or at the end - either way, console.log will be called.
To add an element to the end of an array in JS, it's easier to use push:
array1.push(x);

You might consider using more precise variable names, to make the code more readable. (I'd avoid using single-letter variable names except maybe for i, which is pretty universally understood to be an index.)

const linearSearchSentinel = (array, itemToFind) => {
    const { length } = array;
    array.push(itemToFind);
    for (let i = 0;; i++) {
        if (array[i] === itemToFind) {
            array.pop(); // remove the added item from array
            if (i === length) {
                return console.log('NOT FOUND');
            }
            return console.log('FOUND ON INDEX', i);
        }
    }
};

linearSearchSentinel(['foo', 'bar', 'baz'], 'foo');
linearSearchSentinel(['foo', 'bar', 'baz'], 'baz');
linearSearchSentinel(['foo', 'bar', 'baz'], 'nope');

Or, if you wanted to use the while approach, then don't put any testing inside the loop - simply increment the index:

const linearSearchSentinel = (array, itemToFind) => {
    const { length } = array;
    array.push(itemToFind);
    let i = 0;
    while (array[i] !== itemToFind) i++;
    array.pop(); // remove the added item from array
    if (i === length) {
        return console.log('NOT FOUND');
    }
    console.log('FOUND ON INDEX', i);
};

linearSearchSentinel(['foo', 'bar', 'baz'], 'foo');
linearSearchSentinel(['foo', 'bar', 'baz'], 'baz');
linearSearchSentinel(['foo', 'bar', 'baz'], 'nope');

It's also a good idea to prefer const over let when you don't need to reassign the variable name.
I'd prefer one of the easy-to-understand versions above, but if you really want to avoid changing the length of the array for optimization purposes, I suppose you could replace the last item similar to you're originally doing instead.

const linearSearchSentinel = (array, itemToFind) => {
    const { length } = array;
    const lastItem = array[length - 1];
    array[length - 1] = itemToFind;
    let i = 0;
    while (array[i] !== itemToFind) i++;
    array[length - 1] = lastItem;
    console.log(
        (i !== length - 1 || lastItem === itemToFind)
            ? 'FOUND ON INDEX ' + i
            : 'NOT FOUND'
    );
};

linearSearchSentinel(['foo', 'bar', 'baz'], 'foo');
linearSearchSentinel(['foo', 'bar', 'baz'], 'baz');
linearSearchSentinel(['foo', 'bar', 'baz'], 'nope');

This is all for informational purposes only, of course - if you were actually facing a practical problem where you needed to find the index of an element in an array in JavaScript, it would make more sense to use the built-in indexOf method, which would be far faster and easier. Better not to reinvent the wheel unless an assignment like this one requires it.