Three functions to select a random character, each of which can be used at most twice I'm attempting to minimise the "duplicate" functions I have defined which do almost the exact same thing, but I'm unsure of the best way to do it, I thought of passing in the type I'm looking for (e.g. 1 for GreyLeader, 2 for HeadMarshal etc.) but that ended up looking too messy for my liking.

Could you guys perhaps suggest a way to combine the three functions into one to return the same result?

Some notes:
 - marshal is a list of a Marshal class which has Name, HeadMarshal, Marshal, GreyLeader boolean flags as properties
 - usedMarshals is a list of Marshals
 - each marshal can only be used twice, so the usedMarshals.count(marshal) = 2 is used for that

Code:

def get_marshal():
    marshal = random.choice(marshals)
    if marshal.Marshal:
        if not marshal in usedMarshals:
            return marshal
        elif marshal in usedMarshals:
            if usedMarshals.count(marshal) < 2:
                return marshal
            elif usedMarshals.count(marshal) >= 2:
                return get_marshal()
    elif not marshal.Marshal:
        return get_marshal()

def get_head_marshal():
    marshal = random.choice(marshals)
    if marshal.HeadMarshal:
        if not marshal in usedMarshals:
            return marshal
        elif marshal in usedMarshals:
            if usedMarshals.count(marshal) < 2:
                return marshal
            elif usedMarshals.count(marshal) >= 2:
                return get_head_marshal()
    elif not marshal.HeadMarshal:
        return get_head_marshal()

def get_grey_leader():
    marshal = random.choice(marshals)
    if marshal.GreyLeader:
        if not marshal in usedMarshals:
            return marshal
        elif marshal in usedMarshals:
            if usedMarshals.count(marshal) < 2:
                return marshal
            elif usedMarshals.count(marshal) >= 2:
                return get_grey_leader()
    elif not marshal.GreyLeader:
        return get_grey_leader()

Iterative instead of recursive

Python doesn't really suit itself for recursion

No tail recursion
Generally slower
Limited max depth

Use ALL_CAPS for constant variables

marshels is a constant variable and should be written in ALL CAPS

Thnx @Gareth Rees for clarifying
Functions and variables should be snake_case
@Caridorc is correct in saying that something like isinstance() might work, but without the actual class it is difficult to tell
Think about changing the data type of usedMarshels

list.count(x) is an \$O(n)\$ operation, see time complexity, meaning it will have to loop through the entire list to find how many there are.

Instead you could hold a dictionary of the amount of marshals making it an \$O(1)\$ operation.

Here is some code to give you an idea hopefully,

import random

# Random Marshall class
class Marshel():
    def __init__(self, name):
        self.name = name

    def __str__(self):
        return self.name

a = Marshel("First Marshal")
b = Marshel("Second Marshal")

MARSHELS = [a, b]
used_marshals = {a: 2, b: 1}

def get_leader(type_):
    while True:
        # marshal = random.choice((m for m in MARSHELS if isinstance(m, type_)))
        marshal = random.choice(MARSHELS)
        if marshal not in used_marshals or used_marshals[marshal] < 2:
            return marshal

if __name__ == "__main__":
    print(get_leader("Does nothing yet"))
    # Will print "Second Marshal" because that is the only viable option


Encapsulating logging and throwing exceptions into a method. Bad practice? original code example

if (var1 == null) {
    String msg = "var 1 is null";
    logger.log(msg);
    throw new CustomException(msg);
}
if (var2 == null) {
    String msg = "var 2 is null";
    logger.log(msg);
    throw new CustomException(msg);
}
if (var3 == null) {
    String msg = "var 3 is null";
    logger.log(msg);
    throw new CustomException(msg);
}
return var3.toString(); // ok

new code

if (var1 == null) {
    logMessageAndThrowException("var 1 is null");
}
if (var2 == null) {
    logMessageAndThrowException("var 2 is null");
}
if (var3 == null) {
    logMessageAndThrowException("var 3 is null");
}
return var3.toString(); // NetBeans warn me here for a NPE

void logMessageAndThrowException(String msg) throws CustomException {
    logger.log(msg);
    throw new CustomException(msg);
}

Is this a bad practice or NetBeans just are not able to understand the code?

The second version removes some duplication, so it's better, but usually you should choose one of logging and rethrowing. Frameworks/appservers usually catch and log uncatched exceptions so you will have two (or more) entries for the same exception it the log file which makes debugging hard.

  Log the exception only once!

  One of the common pain points that I have noticed is logging and
  rethrowing an exception. As a result, the log files contains the same
  exceptions several times on several stack levels.

Author: Nayaki


Haskell grep simplification I am new in haskell and for start I choosed to write simple grep. Now I want to ask if there is some simplier/shorter way to write it. For example if there is any way to avoid recursion.

parseLines :: String -> [String] -> Int -> IO ()
parseLines _ [] _ = return ()
parseLines pattern (x:xs) line = do
    when (isInfixOf pattern x) $ putStrLn $ (show line) ++ ": " ++ x
    parseLines pattern xs (line+1)

processFile :: String -> String -> IO ()
processFile _ [] = return ()
processFile pattern file = do
    exists <- doesFileExist file
    if not exists
        then putStrLn $ file ++ ": file does not exists"
        else do
            putStrLn file
            content <- readFile file
            parseLines pattern (lines content) 0

processFiles :: String -> [String] -> IO ()
processFiles _ [] = return ()
processFiles pattern (x:xs) = do
    processFile pattern x
    processFiles pattern xs

main = do
    args <- getArgs
    processFiles (head args) (tail args)

I see these areas where your code could be improved:

processFiles can be expressed very simply using mapM_ from Control.Monad:

processFiles :: String -> [String] -> IO ()
processFiles pattern = mapM_ (processFile pattern)

All your functions are in the IO monad. This goes a bit against Haskell's philosophy to keep side effects to minimum.
parseLines requires the whole file to be read into the memory. This could be solved by using lazy IO, but I'd strongly discourage you from doing so.

One possibility to solve 2. and 3. is to use conduits. This may seem as somewhat complex subject, but the idea is actually very intuitive. A conduit is something that reads input a produces output, using some particular monad. This allows to break your program into very small, reusable components, each doing a single particular task. This makes it easier to debug, test and maintain.

For example, your code could be refactored as follows. (First some required imports.)

import Control.Monad
import Control.Monad.IO.Class
import Data.ByteString (unpack)
import Data.Conduit
import qualified Data.Conduit.Binary as C
import qualified Data.Conduit.List as C
import Data.List (isInfixOf)
import System.Environment (getArgs)
import System.Directory (doesFileExist)
import System.IO

sourceFileLines :: (MonadResource m) => FilePath -> Source m String
sourceFileLines file = bracketP (openFile file ReadMode) hClose loop
  where
    loop h = do
        eof <- liftIO (hIsEOF h)
        unless eof (liftIO (hGetLine h) >>= yield >> loop h)

This function takes a file name and creates a Source - a conduit that takes no input, but produces output. It reads a file line by line and sends each line down the pipeline using yield. Using bracketP we ensure that the file will get closed no matter what happens to the pipeline.

numLines :: (Monad m) => Conduit a m (Int, a)
numLines = C.scanl step 1
  where
    step x n = (n + 1, (n, x))

This component built using scanl is very simple. It just sends its input to the output, and keeps the count along the way. Notice that this conduit doesn't need any IO, it works with any monad.

Now it's easy to filter a stream of numbered lines with a pattern:

parseLines :: (Monad m) => String -> Conduit String m (Int, String)
parseLines pattern = numLines =$= C.filter f
  where
    f (_, x) = isInfixOf pattern x

This function fuses two conduits together. The first one numbers lines, the second filters them according to the pattern.

printMatch :: (MonadIO m) => Sink (Int, String) m ()
printMatch = C.mapM_ (\(n, x) -> liftIO $ putStrLn $ show n ++ ": " ++ x)

In printMatch we separate the logic that prints out the output. For each pair it receives it prints the line number and its content.

Combining and running these conduits is then easy:

runResourceT $ sourceFileLines file $= parseLines pattern $$ printMatch

(runResourceT is needed because of bracketP.) So the rest of the program would look like

processFile :: String -> String -> IO ()
processFile _ [] = return ()
processFile pattern file = do
    exists <- doesFileExist file
    if not exists
        then putStrLn $ file ++ ": file does not exists"
        else do
            putStrLn file
            runResourceT $ sourceFileLines file $= parseLines pattern $$ printMatch

processFiles :: String -> [String] -> IO ()
processFiles pattern = mapM_ (processFile pattern)

main = do
    args <- getArgs
    processFiles (head args) (tail args)


Collate a growing number of String arrays and search through them These string arrays are used to classify whether the incoming feed/message is a driver feed or team feed.

private static final String[] DRIVER_FEED_TYPES = {
        "SEBASTIAN_VETTEL", "LEWIS_HAMILTON", "CHARLES_LECLERC", "MAX_VERSTAPPEN",
};

private static final String[] TEAM_FEED_TYPES = {
        "FERRARI", "MERCEDES", "SAUBER", "REDBULL"
};

This method gets the type of feed through the incoming message and determines the corresponding type through the arrayContains method.

private boolean isFeedEnabled(FormulaOneMessage formulaOneMessage) {
    String feedType = formulaOneMessage.getFeedType();

    if (Helper.arrayContains(DRIVER_FEED_TYPES, feedType)) {
        return isDriverFeedEnabled();
    } else if (Helper.arrayContains(TEAM_FEED_TYPES, feedType)) {
        return isTeamFeedEnabled();
    } else {
        return false;
    }
}

The arrayContains method basically looks through the Object array to see if the object is there.

public static boolean arrayContains(Object[] list, Object item) {
    for (Object s : list) {
        if (s.equals(item)){
            return true;
        }
    }

    return false;
}

The challenge is how to manage the growing number of types of String arrays in the class. Should I make it into a map, stream or an enum?

Also, the if else statement would also grow as I continue to add more FEED_TYPES. Should I continue to use them?

Overall, I wanted to improve the maintainability of this code seeing the number of feed types will continue to grow in the future.

You should use Sets instead of Arrays and Enums when it makes sense. I would write that as:

enum FeedClass {
    TEAM, DRIVER, OTHER;
    private static final Set<String> DRIVER_FEED_TYPES = new HashSet<>(Arrays.asList(
            "SEBASTIAN_VETTEL", "LEWIS_HAMILTON", "CHARLES_LECLERC", "MAX_VERSTAPPEN"));

    private static final Set<String> TEAM_FEED_TYPES = new HashSet<>(Arrays.asList(
            "FERRARI", "MERCEDES", "SAUBER", "REDBULL"
    ));

    public FeedClass fromType(String feedType) {
        if (DRIVER_FEED_TYPES.contains(feedType)) {
            return DRIVER;
        } else if (TEAM_FEED_TYPES.contains(feedType)) {
            return TEAM;
        } else {
            return OTHER;
        }
    }
}

private boolean isFeedEnabled(FormulaOneMessage formulaOneMessage) {
    FeedType feedType = FeedClass.fromType(formulaOneMessage.getFeedType());
    switch (feedType.getClazz()) {
        case DRIVER:
            return isDriverFeedEnabled();
        case TEAM:
            return isTeamFeedEnabled();
        default:
            return false;
    }
}

Driver names isn't really something that should be hardcoded, you probably want to move it to database or external file(s).
You can't really avoid if/switch in isFeedEnabled() unless you move isDriverFeedEnabled and isTeamFeedEnabled into enum, it might not be
 possible in your code to do it cleanly.


Refactoring if statement based on a hash key I have to call Article.to_draft/to_archive/publish method depending on the presence of the corresponding key in the params hash.

I do, however, have no idea to implement it properly.

def update_state
  method = if params[:draft].present?
             :to_draft
           elsif params[:archived].present?
             :to_archive
           else
             :publish
           end
  @article.send method
end

The code works, but the number of possible states of an article will probably grow in the future.

I would like to have a hash like this:

{ draft: :to_draft, archived: :to_archive, default: :publish }

and let #update_state decide what should be called, based on the params hash.

I suggest using find

ARTICLE_ACTIONS = { :draft => :to_draft,
                    :archived => :to_archive,
                    :default => :publish
                  }

def update_state
  state = [:draft, :archived, :default]
             .find {|state| params[state].present?}
  @article.send ARTICLE_ACTIONS[state]
end


Union based encoding of .csv file into a data table with different types I am writing a program that has a row-based data table class. It also has an encoder class that is intended to take in some .csv data and convert it into a 2darray of encoded data of unknown type at compile time.

The idea is that the user will point to a .csv file they would like to read from and specify a runtime schema (column 1 == int, column 2 == bool, column 3 == string, etc) that will be used to encode the raw .csv data into the proper types needed for comparison and such elsewhere in the program.

I have been reading a lot about this and decided to try a union based approach.
I mocked this up in a simple project to see how this might work out on one single piece of data ("DataPoint."). Here is the code.

#include "pch.h"
#include <iostream>
#include <sstream>

union DataPoint
{
    bool Bool;
    int  Integer;
    float FloatingPoint;
    //std::string String;
};

enum DataTypes
{
    Bool,
    Integer,
    FloatingPoint,
    //String
};

DataPoint EncodeData(DataPoint InDataPoint, DataTypes InDataTypes, std::string InData)
{
    std::stringstream DataStream(InData);

    switch (InDataTypes)
    {
    case Bool:
        std::cout << "Type is Bool" << std::endl;
        if (InData == "True" || InData == "true"  || InData == "1") InDataPoint.Bool = true;
        else if (InData == "False" || InData == "false" || InData == "0") InDataPoint.Bool = false;
        else std::cout << "This data is not compatable with the bool type." << std::endl;
        break;
    case Integer:
        std::cout << "Type is Integer" << std::endl;
        DataStream >> InDataPoint.Integer;
        break;
    case FloatingPoint:
        std::cout << "Type is Floating Point" << std::endl;
        DataStream >> InDataPoint.FloatingPoint;
        break;
    /*
    case String: InDataPoint.String = InData;
        break;
    */
    default:
        break;
    }

    return InDataPoint;
}

void PrintData(DataPoint InDataPoint, DataTypes InDataTypes)
{
    switch (InDataTypes)
    {
    case Bool: std::cout << "Encoded data value : " << InDataPoint.Bool << std::endl;
        break;
    case Integer: std::cout << "Encoded data value : " << InDataPoint.Integer << std::endl;
        break;
    case FloatingPoint: std::cout << "Encoded data value : " << InDataPoint.FloatingPoint<< std::endl;
        break;
    /*
    case String:
        break;
    */
    default:
        break;
    }
}

int main()
{
    DataPoint DataPoint1{};
    DataTypes DataType{ Bool };
    std::string Data{ "true" };

    std::cout << "Raw data value : " << Data << std::endl;

    PrintData(EncodeData(DataPoint1, DataType, Data), DataType);

    return 0;
}

This all seems to work, however, as you can see... I have commented out string entries because I cannot figure out a good way to handle them. One issue I run into is though unions apparently support strings in c++ now, I error out when I try to add a string as a union member. Here is the error I get if I uncomment the string member of the union.

Severity   Code    Description Project File    Line    Suppression State
Error (active)  E1776   function "DataPoint::DataPoint(const DataPoint &)" (declared implicitly) cannot be referenced -- it is a deleted function

Is there some way to handle the string?
Also, in general, does this approach seem sound?

I don't have time to check all now but I found some things I want to share

If you are using C++17 consider using std::variant instead of the weak union inherited from c. It will probaly also solve youre isue with std::string. See: https://en.cppreference.com/w/cpp/utility/variant

Consider using '\n' instead of std::endl. Im pretty sure you only want a newline and not also a expensive flush of the buffer: https://stackoverflow.com/questions/213907/c-stdendl-vs-n.

Consider using the more safe enum class instead of plain c enum. It is available since C++11.


Magic Static in Singleton Template I recently was reading an article labeled "Modern C++ Singleton Template". The proposed solution used C++11 feature of Magic Statics (N2660). I wanted to adapt it for an own project in
"freestanding" (fno-hosted) environment.
My code looks as follows:
Singleton.hpp
namespace std
{
    template <typename T>
    class Singleton
    {
        public:
            // = initialization and termination methods
            Singleton( const Singleton& )   = delete;   // copy constructor
            Singleton(       Singleton&& )  = delete;   // move constructor
            Singleton& operator=( const Singleton& ) = delete;  // assignment operator
            Singleton& operator=(       Singleton&&) = delete;  // copy move operator
            // = accessor methods.
            static T& getInstance();
        protected:
             Singleton() {};                            // default constructor
            ~Singleton() {};                            // default destructor
             struct Token {};
        private:
    }; // end template class T

    template <typename T>
    T& Singleton<T>::getInstance()
    {
        static T    __singleInstance( Token{} );
        return( __singleInstance );
    } // end public method 'getInstance()'
} // end namespace std

Test.hpp
#include "libs/libc++/singleton.hpp"        // declaration of template class 'Singleton'

class Test final : public TestSingelton<Test>
{
    public:
    Test(TestToken);
   ~Test();

    void use();
};

Test.cpp
Test::Test(TestToken)
{
    logTraceEvent_m( loging to file );
}

Test::~Test()
{
    logTraceEvent_m( loging to file );
}

void
Test::use()
{
    logTraceEvent_m( loging to file );
}

Problem
In hosted environment (Linux, GCC 7.5.0) it works as presented in the article refereed to above.
The static variable is initialized only once; the constructor of the derived class is called only once.
Entering main()
Entering a()
constructed
in use
Entering b()
in use
Leaving main()
destructed

However, in my "-ffreestandig" environment it is not working.
"freestanding" = kernel without any standard headers; without any libraries!
Static variable ("__singleInstance") is initialized on every call to "Test::getInstance()".
______ TRACE: constructor = Test::Test(TestSingelton<Test>::TestToken), ...
______ TRACE: use = void Test::use(), file = test/Test.cpp, line = 37.
______ TRACE: constructor = Test::Test(TestSingelton<Test>::TestToken), ...
______ TRACE: use = void Test::use(), file = test/Test.cpp, line = 37.

What I'm doing wrong?
Is there any advice in which direction I have to investigate?

compiler options?
-W -Wall -pedantic -ffreestanding -std=gnu++17 -fno-PIC -fno-exceptions -fno-rtti
-fno-use-cxa-atexit -march=native -m64 -mcmodel=kernel -mno-red-zone -mno-sse3 -mno-ssse3
-mno-sse4.1 -mno-sse4.2 -mno-sse4 -mno-sse4a -mno-3dnow -mno-avx -mno-avx2
runtime environment, that is missing something

Your test code doesn't match your test output. "Test.cpp" isn't even valid C++ code!
How does your massive amount of code do anything significantly different from the following eight lines?
template<class T>
struct Singleton {
    struct Token {};
    static T& getInstance() {
        static T instance(Token{});
        return instance;
    }
};

Example usage:
struct Widget { explicit Widget(Singleton<Widget>::Token); };
Widget& example = Singleton<Widget>::getInstance();

As for why the "thread-safe statics" feature doesn't work on your freestanding platform, I would suspect it's because your freestanding platform doesn't support thread-safe statics. :)  Check the documentation for your toolchain to see how static variables are handled on that particular platform.


Implementing a Sort for an Array Yesterday, I gave an interview after a long time, and I was asked a question which I couldn't solve on the spot.

I had to implement a sort function for a Array of strings in JavaScript, with the following requirements:

The Array will always contain Strings
For sorting, Upper Case has higher priority than Lowe Case Letters, with Numbers having the Least.
Assume that Array.sort() cannot be used
The performance of the sorting is not a factor; i.e. Bubble sort is fine.

I was thinking of the solution today, and came up with this:

function sortArray(items) {
  let itemcount = items.length;
  for (var i = 0; i < itemcount; i++) { //number of passes
    for (var j = 0; j < (itemcount - 1 - i); j++) { //for checking all pairs in a given pass
      let a = items[j];
      let b = items[j + 1];
      if (compareCaseSenitiveStrings(a, b) < 0) {
        console.log("Switch", a, b)
        items[j + 1] = a;
        items[j] = b;
      }
    }
  }
  //return
  return items;
}

//Function to compare two given strings, and return the index of the higher priority one
function compareCaseSenitiveStrings(a, b) {

  //Inner functions
  function compareCharacters(a, b) {
    //check the case first()
    if (a.getFirstCase() == b.getFirstCase()) {
      let charDiff = a.codePointAt(0) - b.codePointAt(0);
      return -Math.sign(charDiff)
    } else {
      //different cases
      //We need to pass the id of the one which is higher;
      let requiredCaseOrder = ["NONE", "LOWER", "UPPER"];
      return requiredCaseOrder.indexOf(a.getFirstCase()) - requiredCaseOrder.indexOf(b.getFirstCase());
    }
  }

  function isNumeric(n) {
      return !isNaN(parseFloat(n)) && isFinite(n);
    }
    //End of inner functions
    //===============================================

  //Start of function here

  //First check if both are numbers
  if (isNumeric(a) && isNumeric(b)) {
    //return the index of smaller number (since it has higher priority)
    return -Math.sign((parseFloat(a) - parseFloat(b)));
  }

  //Now check that lengths of both is greater than 0
  if ((a.length > 0) && (b.length > 0)) {
    //check the first elements of this string
    let result = compareCharacters(a[0], b[0]);
    if (result != 0) {
      //this means one input has higher priority, lets return that
      return result;
    } else {
      //both are equal
      //get the result of remaining characters
      return compareCaseSenitiveStrings(a.substring(1), b.substring(1));
    }
  } else {
    //length of atleast one of these strings is zero.
    //return the index of the longer element
    let lengthDiff = a.length - b.length;
    return Math.sign(lengthDiff);
  }
}

String.prototype.getFirstCase = function() {
  let caseEnum = ["LOWER", "NONE", "UPPER"];
  var caseIndex = 1;
  if (this == this.toLowerCase()) {
    caseIndex--;
  }
  if (this == this.toUpperCase()) {
    caseIndex++;
  }
  return caseEnum[caseIndex];
};

Is this code readable enough?
Are there any other Design Patterns that could be applied? Any other points which could be improved?

Sorting is hard.

console.log( sortArray([ '123', '9' , '1' , '11' ]) ); returns ["1", "9", "11", "123"], that does not sound good. After removing the numeric test I get ["11", "123", "1", "9"] which is better but not intuitive, I would have expected the 1 up front. This shows that you cannot just write the code, you need to build a set of tests to validate your code. Especially during interviews.
String.prototype.getFirstCase is very cute, but it needs commenting for lesser readers.
You are mixing var and let, I am not too excited about that
If you remove the numeric test, then you no longer need isNumeric as well
compareCaseSenitiveStrings should be compareCaseSensitiveStrings
I would convert 2 line comments to 1 line comment for readability, and an overall review of what comments really add value
Other than the code was very readable


Calculating unknown parts of a right triangle I am in the process of creating a program that allows a user to input known parts of a right triangle and the calculates all of the unknown parts if it is possible.

I am using a class that I wrote in order to do the calculations. I wanted to know if there is a more efficient solution to this problem or if my code is good as is.

public class RightTriangle
{
    private double sideA; //Leg
    private double sideB; //Leg
    private double sideC; //Hypotenuse

    private double alpha;
    private double beta;

    private int numOfSides;
    private int numOfAngles;

    public RightTriangle(double a, double b, double c, double A, double B)
    {
        numOfSides=0;
        numOfAngles=0;

        sideA=a; if(sideA>0){numOfSides++;}
        sideB=b; if(sideB>0){numOfSides++;}
        sideC=c; if(sideC>0){numOfSides++;}

        alpha=A; if(alpha>0){numOfAngles++;}
        beta=B; if(beta>0){numOfAngles++;}

        if(numOfAngles<2)
        {
            findMissingAngles();
        }

        if(numOfSides<3)
        {
            findMissingSides();
        }

        if(!isPossible())
        {
            sideA=sideB=sideC=alpha=beta=0;
        }
    }

    private void findMissingAngles()
    {
        if(alpha==0)
        {
            calcAlpha();
        }

        if(beta==0)
        {
            calcBeta();
        }
    }

    private void findMissingSides()
    {
        if(sideA==0)
        {
            calcSideA();
        }

        if(sideB==0)
        {
            calcSideB();
        }

        if(sideC==0)
        {
            calcSideC();
        }
    }

    private void calcAlpha()
    {
        if(numOfAngles==1)
        {
            alpha=90-beta;
        }

        else
        {
            if(sideA>0 && sideB>0)
            {
                alpha=Math.toDegrees(Math.atan(sideA/sideB));
            }

            else if(sideB>0 && sideC>0)
            {
                alpha=Math.toDegrees(Math.acos(sideB/sideC));
            }

            else if(sideA>0 && sideC>0)
            {
                alpha=Math.toDegrees(Math.asin(sideA/sideC));
            }
        }
    }

    private void calcBeta()
    {
        beta=90-alpha;
    }

    private void calcSideA()
    {
        if(sideC>0)
        {
            sideA=Math.sin(Math.toRadians(alpha)) * sideC;
        }

        else if(sideB>0)
        {
            sideA=Math.tan(Math.toRadians(alpha)) * sideB;
        }
    }

    private void calcSideB()
    {
        sideB=sideA/Math.tan(Math.toRadians(alpha));
    }

    private void calcSideC()
    {
        sideC=sideA/Math.sin(Math.toRadians(alpha));
    }

    private boolean isPossible()
    {
        return ((alpha+beta+90)==180 && Math.abs(Math.sqrt(Math.pow(sideA, 2) + Math.pow(sideB, 2)) - sideC) < 1);
    }

    public void printInfo()
    {
        System.out.println("Triangle Info\n");
        System.out.println("Side A: " + String.format("%.02f", sideA));
        System.out.println("Side B: " + String.format("%.02f", sideB));
        System.out.println("Side C: " + String.format("%.02f", sideC) + "\n");
        System.out.println("Alpha: " + String.format("%.01f", alpha));
        System.out.println("Beta: " + String.format("%.01f", beta));
    }
}

Input

Enter Triangle Information (Enter 0 for missing parts)
Enter side A: 60
Enter side B: 60
Enter side C: 0

Enter angle Alpha: 0
Enter angle Beta: 0

Output

Triangle Info

Side A: 60.00
Side B: 60.00
Side C: 84.85

Alpha: 45.0
Beta: 45.0

Keep it simple

        numOfSides=0;
        numOfAngles=0;

        sideA=a; if(sideA>0){numOfSides++;}
        sideB=b; if(sideB>0){numOfSides++;}
        sideC=c; if(sideC>0){numOfSides++;}

        alpha=A; if(alpha>0){numOfAngles++;}
        beta=B; if(beta>0){numOfAngles++;}

        if(numOfAngles<2)
        {
            findMissingAngles();
        }

        if(numOfSides<3)
        {
            findMissingSides();
        }

This seems more complicated than necessary.  Why have numOfAngles and numOfSides at all?  You could just say

        sideA = a;
        sideB = b;
        sideC = c;

        alpha = A;
        beta = B;

        findMissingAngles();

        findMissingSides();

And then modify

        if(numOfAngles==1)

in calcAlpha to say

        if (beta > 0)

That saves two object fields that exist just for construction purposes.

Be careful of false modularity

    private void calcBeta()
    {
        beta=90-alpha;
    }

This only works if you calculate alpha first.  That's the programming logic here, but nothing enforces it.  So why have a separate method to do an assignment based on one subtraction?  You could change

        if(beta==0)
        {
            calcBeta();
        }

to say

        if (beta==0)
        {
            beta = 90 - alpha;
        }

instead.  Then you could get rid of calcBeta.

This also avoids the problem of calling calcBeta without testing that beta doesn't already have a value.

Alternately, you could change calcBeta to be more like calcAlpha where it calculates alpha from beta if beta exists or from two sides otherwise.

Similarly, all the calcSide methods assume that alpha exists.  And calcSideB and calcSideC assume that sideA exists.  If any of that is not true, then you'll get weird results.

Exceptional circumstances

    private boolean isPossible()
    {
        return ((alpha+beta+90)==180 && Math.abs(Math.sqrt(Math.pow(sideA, 2) + Math.pow(sideB, 2)) - sideC) < 1);
    }

Why not change (alpha+beta+90)==180 to just (alpha + beta) == 90?

Why is 1 the acceptable error?  Why not .1 or .03 or something else?

Are negative values acceptable?  Because you accept them if they have the right magnitude.  For example, -95 and 185 are a valid pair of angles.  And -3, -4, 5 are valid side values.


Querying the probability of choosing even or odd elements in a subarray I have written a code where I have an array and I need to print the probability of picking an even/odd number in the given range in the array.

If probability is 0 or 1, I have to print so. And if the probability is say 2/4, I have to reduce it to minimal form and print the output as "1 2" which means 1/2. I have written the below code which handles all these perfectly. But it is taking more than 3 seconds to execute so it is resulting in a timeout as the given time limit is 3 seconds for this.

How do I optimize this further?

#include<stdio.h>
int gcd(int u,int v)
{
int shift;
if(u==0) return v;
if(v==0) return u;
shift = __builtin_ctz(u | v);
u >>= __builtin_ctz(u);
do {
    v >>= __builtin_ctz(v);
    if(u>v) {
        unsigned int t=v;
        v=u;
        u=t;
    }
    v = v-u;
} while(v!=0);
return u << shift;
}
void query(char *arr, int qtype, int l, int r, char v)
{
int q=r-l+1;
int g1=1,g2=1;

int peven=0,podd=0;
for(int i=l-1;i<r;i++)
{
    if(qtype==2 && v)
        arr[i]=(arr[i]^v);
    if(qtype == 1)
    {
        if(arr[i])
        podd++;
    }
    else if(qtype == 0)
    {
        if(!(arr[i]))
        peven++;
    }
}

switch(qtype)
{
case 0:
{
    if(peven == 0)
        printf("0\n");
    else if(peven == q)
        printf("1\n");
    else
    {
        g1=gcd(peven,q);
        if(g1!=1)
        {
            peven/=g1;
            q/=g1;
        }
        printf("%d %d\n",peven,q);
    }
    break;
}

case 1:
{
    if(podd == 0)
        printf("0\n");
    else if(podd == q)
        printf("1\n");
    else
    {
        g2=gcd(podd,q);
        if(g2!=1)
        {
            podd/=g2;
            q/=g2;
        }
        printf("%d %d\n",podd,q);
    }
    break;
}
default:
    break;
}

}

int main()
{
int T;

scanf("%d",&T);

while(T--)
{
    int arr_size;
    scanf("%d",&arr_size);

    int q;
    scanf("%d",&q);

    char *arr=(char *)malloc(arr_size * sizeof(char));
    for(int i=0; i<arr_size;i++)
    {
        int x;
        scanf("%d",&x);
        arr[i]=(char)(x&1);

    }
    while(q--)
    {
        int qtype,l,r;
        char v;

        scanf("%d%d%d",&qtype,&l,&r);

        if(qtype==2)
        {
            int y;
            scanf("%d",&y);
            v=(char)(y&1);

        }

        query(arr,qtype,l,r,v);
    }
    free(arr);
}
return 0;
}

  Format of the input file:

  First line : T i.e Number of testcases.
  For each testcase:

  First line : Two space separated integers N and Q.
  Second line : N space separated integers denoting the array.
  Next Q lines ; Space separated integers representing a query.

  Queries can be:

  0 L R :- You have to find and print the probability of choosing an even number from the segment [L,R] (both Inclusive).
  1 L R :- You have to find and print the probability of choosing an odd number from the segment [L,R] (both Inclusive).
  2 L R K :- You have to add number K to all the elements in the range, changing the values for subsequent queries

  Format of the output file:

  Output the answer to each query in a separate line.

  Constraints:

  1<= N,Q,K <= 1000000

  Sample input;

1
5 3
6 5 2 1 7
0 2 2
2 2 5 3
1 2 5

  Sample Output:

0
1 4

  Explanation: For query 3 numbers in the range [2,5] are {8,5,4,10}
  Hence probability of choosing a odd number is 1/4.

Well, first you should properly format your code. Readability means maintainability, and fewer bugs.

Try to avoid type-conversions and casts. They are danger-points, and if you use a decent warning-level, your compiler should warn about potentially dangerous type-conversions at least.
In gcd() you use int everywhere but once, where you use unsigned int. Fix that.

You have three query-types, and you seem to desperately want to make them share code. Don't do that! Branch-misprediction is expensive, and moving work out of inner loops is always a win. Hopefully, your compiler will do it for you, so the compiled code does not suffer from your choice like the source code does.

Take a look at which compound-assignment-operators are available. They make assignments more readable and concise.

Try to declare your variables in the smallest scope you can for better readability. Even if you insist on strict C89 with GCC builtins, you can go further.

Don't assume success! scanf() and malloc() can fail, so check for it.

Don't cast the result of malloc(). It's unnecessary and error-prone.

Normalize the range early-on for ease of use. A pointer and a length are sufficient unless you need the position in the surrounding array, and you don't.

You might want to experiment with using a bit-vector, or representing the array as spans of odd / even numbers.


Throw exception in favor of an error? I've been reviewing my previous projects for the last few days. I've found that I never return an error from any of my methods. I always throw exceptions. I googled about exceptions vs errors and I feel like I should return an error if there's an error.

Please consider my service class from my previous project. We have online tutorials on our website and we send out a token (sort of a coupon) to users time to time. Users can redeem the token to get a full access to a certain tutorial on our website.

class TutorialTokenRedeemptionService {

    protected $memberRepository;
    protected $tutorialRepository;
    protected $tokenRepository;

    public function __construct(
        Repository\TutorialRepositoryInterface $tutorialRepository,
        Repository\TokenRepositoryInterface $tokenRepository,
        Repository\MemberRepositoryInterface $memberRepository
    )
    {

        $this->memberRepository   = $memberRepository;
        $this->tutorialRepository = $tutorialRepository;
        $this->tokenRepository = $tokenRepository;
    }

    public function redeem($tokenCode, Entity\MemberInterface $member)
    {

        // Let's see if we have the token in our system
        $token = $this->tokenRepository->findByCode($token);

        if( !$token ) {
            throw new Exception\TokenNotFound($token);
        }

        // Let's see if we have the associated tutorial
        $tutorial = $this->tutorialRepository->find($token->getBatch()->tutorial_id);

        //
        if( !$tutorial ) {
            throw new Exception\TutorialNotFound($token);
        }

        $member->addPurchasedTutorial($tutorial);
        $this->memberRepository->save($member);

        $token->setActivatedBy($member->id);
        $this->tokenRepository->save($token);

        return new TutorialTokenRedeemptionServiceResult(StatusCodes::OK, $member, $tutorial, $token);

    }

}

It might be considered an exception to users if the token he/she is trying to use is not in our system because users might have more than one token. Users can try other tokens. From the perspective of the TutorialTokenRedeeptionService and its client, it's an error because the service can't continue anymore. Am I going the right way?

What should be better? Throw exceptions all the time or throw exceptions and return errors depending on use cases?

Well, I'd say that, apart from one or two things, you're going about it in exactly the right way.
A class, and all of its methods process data. If the data supplied poses an exceptional problem (ie, the data is invalid), then an Exception has to be thrown back to the code that passed that data.

The motto is: He who supplies the data, has to handle the errors.
By having methods simply return errors, your class is taking on too many responsibilities: Process data and error handling? No, that is in violation of the single most important SOLID principle: the SRP (Single Responsability Principle).

Here's why I am prepared to defend my statements on throwing exceptions from classes:

A class cannot check if the calling code is checking the return value of its method. You may return false or ERROR, but if the caller doesn't check it, you might aswell return a Shakespeare sonnet...
If you receive bad data, don't try to make sense of it, or don't try to make it work: the calling code has a bug in it, and should be fixed. The best way to spot the troubled bit of code is to throw new InvalidArgumentException and look at the stack-trace. Like I said before: bad arguments received often means bad code is calling your method: FIX IT.
If your class is well-tested, then it's safe to say that if ever it encounters a scenario where it can't do its job properly, something quite Exception-al has happened, treat it accordingly.
returning error-indicating values makes unit-testing harder, in a way. I, personally, like the odd @expectedException annotation, for example. If the test fails, it also yields more speceific output anyway.
You don't know what code is going to handle the problem. Returning an error-indicator is all well and good, provided that return value is caught, and indeed returned all the way through to the code that will eventually deal with it. An exception just keeps going until it is caught. If not, the whole application just grinds to a halt. Either way, returning false is less pressing, there's no real incentive to do something. An exception, however, must be handled for the program to go on.
You cannot register handlers for return false; or return null; statements, but you can register error and exception handlers. And you will do so in larger projects. This, certainly, is a huge advantage of exceptions.
Returning errors depending on the usage of your class limits the context/use-cases, in which you can use the class. A class should be a self-contained, portable unit of code. It should, therefore, just process data, or handle exceptional cases, not both.

Another quick hang-up of mine that I can't have go unmentioned: Please, please, please subscribe to the coding standards, as described by PHP-FIG.

Other niggles I have, and I'm really nit-picking here:

throw new Exception\TokenNotFound();

Ok, when I see this throw statement, I know that the TokenNotFound class is extending the Exception core-class (I hope), just like all other classes I might find in the Exception namespace. However, writing namespace Exception; at the top of a file could confuse PHP, if it doesn't already. Also, in files using the use keyword, the complementing catch statement just doesn't look right to me:

use Exception\TokenNotFound;
try
{
    //stuff
}
chatch(TokenNotFound $t)

Look at all Exception classes in common use: they all contain the word Exception (or Fault in SoapFault).TokenNotFoundcould be anything from a mock object, to a handler for just such an exceptional case, when in fact it is the exception object itself. Just appendException` to the exception class names. You may find this redundant at first, but at some point in time people using your code, or even yourself, will thank you for it.

use Exception\TokenNotFoundException;
try
{
    //stuff
}
chatch(TokenNotFoundException $t)


Python TCP echo server The below code is a simple TCP echo server. The folks using it claim "it is going down every now and again". Any thoughts on how this could be improved would be really appreciated, particularly if they would improve the reliability.

import select, argparse, socket

def server_socket(ip, port):
    serv = socket.socket()
    try:
        serv.bind((ip, port))
    except:
        if (ip != ''):
            print("Failed to bind to " + ip + " on port " + str(port) + " exiting" )
        else:
            print("Failed to bind to all on port " + str(port) + " exiting" )
        exit()

    if (ip != ''):
        print("Bound to " + ip + " on port " + str(port) + " echoing tcp" )
    else:
        print("Bound to all on port " + str(port) + " echoing tcp" )

    serv.listen(1)
    inputlist = [serv,]

    while 1:
        inputready,_,_ = select.select(inputlist,[],[], 20)

        if not inputready:
            for i in inputlist:
                if i != serv:
                    i.close()
                    inputlist.remove(i)
                    print("connection timeout")

        for i in inputready:
            if i == serv:
                client, address = serv.accept()
                inputlist.append(client)
                print("connection open from " + str(address))
            else:
                try:
                    data = i.recv(1024)
                    print("received " + str(len(data)) + " bytes"),
                    print(" \""+ data +"\"")
                    if data:
                        i.send(data)
                    else:
                        i.close()
                        inputlist.remove(i)
                        print("connection closed")
                except:
                    i.close()
                    inputlist.remove(i)
                    print("connection closed")

if __name__ == '__main__':
    parser = argparse.ArgumentParser(prog = "TCP Echo server")

    parser.add_argument('-i', '--ip', type=str, default='', help = "IP to bind to")
    parser.add_argument('port', type=int, help = "Port to bind to")

    args = parser.parse_args()
    server_socket(args.ip, args.port)

The folks using it claim "it is going down evey now and again".

Well, yes. If they don't send you a message in 20 seconds, you're disconnecting them! Why? Simply removing that block is probably what you want.

Separation of Concerns

Your server_socket function both creates a server and handles the busy loop. You should split those into two functions:

def make_server(ip, port):
    ...

def run_echo(server):
    ...

server = make_server(ip, port)
run_echo(server)

Echoing

You wrap your echo block in a try/except. Why? What part of that code can raise an exception? What exception is raised? You should prefer to only catch precisely the exceptions that are raised (to avoid, for instance, catching SyntaxError and hiding whatever typo you wrote in your code), but in this case I don't know what exception you would be catching.

Also, you don't need to print str(len(data)), you can just print len(data).


Demonstration of a singly linked list This is a simple demonstration of a singly linked list, from my class work:

#include <stdio.h>
#include <stdlib.h>
#include <wctype.h>
#include <errno.h>
#include <assert.h>
#include <ctype.h>
#include <stddef.h>
#include <wchar.h>
#include <locale.h>
#include <setjmp.h>
struct node
{
    int          info;
    struct node *link;
};

typedef struct node* NODE;

NODE main()
{
    NODE first = NULL;
    int  item, choice;

    for(;;)
    {
        printf("1.INSERT AT FRONT\n2.DISPLAY\n3.QUIT\n\nPLEASE ENTER YOUR CHOICE:");
        scanf("%d",&choice);

        switch (choice)
        {
            case 1:
            printf("\nEnter the item to be inserted:");
            scanf("%d", &item);

            first = insert_front(item,first);
            break;

            case 2:
            display(first);
            break;

            case 3:
            exit(0);
        }
    }
}

NODE getnode()
{
    NODE x;

    x = (NODE) malloc(sizeof(struct node));
    if(x == NULL)
    {
        printf("Out of memory");
        exit(0);
    }
    return x;
}

int insert_front(int item, NODE first)
{
    NODE temp;
    temp = getnode();
    temp->info = item;
    temp->link = first;
    return temp;
}

void display(NODE first)
{
    NODE temp;

    if(first == NULL)
    {
        printf("Empty");
        return;
    }

    printf("\nThe contents are: ");

    temp = first;
    while(temp != NULL)
    {
        printf("%d ", temp->info);
        temp = temp->link;
    }
    printf("\n");
}

As you can see, this code demonstrates the working of a linked list. How can this program more smaller, better, more professional and much more good? If you were given a chance to modify this program, how would you do it? I wanna know how to think and how to train myself to modify programs like these. And as you see, I have learned most of C. I referred to and learned C from K.N Kings The C programming language: A modern approach and Computer science: A structured approach using C and data structures from Data structures using C by Tenenbaum.

Now I have stretched my legs in learning C what can I do to create application that might be useful to me or to others. Say I want to create a small game like "snake" or create a small application that my father might use to do his business.

I'm not a C-guru at all but I believe I can give you a few advices to improve your code:

Review your included libraries. Do you really need all those ones? Nope. Chuck Testa You only use stdio.h and stdlib.h.
main() should return int and you specify it actually returns NODE... but it isn't returning nothing at all! Double fail x( So... int main() { return 0; } FTW
Naming consistency. You have written getnode() and insert_front(). You can name your members whatever you want (getnode(), getNode(), get_node()) but keep the patter for all of them (insertfront(), insertFront(), insert_front()). IMO, underscores are much C-style.
Blank lines. Avoid inserting blank lines. They don't clarify the reading of your code, instead of that they expand your code unnecessarily. You can insert blank lines between your function definitions. This helps to delimit and group your functions and eases their location.
What if I type 4 in your menu? The golden rule in programming is: Users provide bad inputs. So you always must to validate inputs. Don't matter if it seems trivial. Always.
And for last and getting a bit cranky, you can get a temp node in one step within insert_front():

NODE temp = get_node(); // I've changed `getnode()` name for you. You are welcome.


How to optimize my CSS/HTML webpage? I am working on a (HTML/CSS) website to improve my skills but I am not sure if I am doing it the right way, because I have a lot of floats and sizes. I also need help with some CSS things:

What I have:

What I am Shooting for

The red dimensions in the image are the dimensions I've tried to give the objects and which I am not sure if it is the correct way of doing it.
The black words are the things I would like to change in the future, but I need this code reviewed first.

All my code:
INDEX.HTML

sidebar

 Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book.

STYLE.CSS

@import url(reset.css);

body {
background:#f1f1f1;
}

#fullHeader {
float:left;
min-width:100%;
height:90px;
}

#header {
min-width:100%;
height:60px;
background:#B52B42;
}

#inner-header {
width:1180px;
height:60px;
margin:0 auto;
}

#home {
float: left;
width: 140px;
}

#menubar{
float: left;
width: 1180px;
}

#wrapper {
width:1180px;
margin:0 auto;
-moz-box-shadow:0 0 5px #888;
-webkit-box-shadow:0 0 5px #888;
box-shadow:0 0 5px #888;
}

#mainContent {
width:100%;
float:left;
background-color:#FFF;
}

#newsHolder {
float:left;
width:840px;
height:493px;
}

#item1 {
float:left;
width:477px;
height:320px;
margin-bottom:5px;
}

#item2 {
float:right;
width:358px;
height:244px;
margin-bottom:5px;
}

#item3 {
float:left;
width:236px;
height:167px;
margin-right:5px;
}

#item4 {
float:left;
width:236px;
height:167px;
}

#item5 {
float:right;
width:358px;
height:244px;
}

#item1 img,#item2 img,#item3 img,#item4 img,#item5 img {
width:100%;
height:100%;
}

#sidebar-right {
float:right;
width:340px;
background:#FFF;
}

#newsList {
float:left;
width: 840px;
background:#FFF;
}

#footer {
float:left;
min-width:100%;
height:70px;
background:#006;
}

I did not post the CSS code of the navigation menu because it is already working correctly.

I would be very happy if anyone can help me make my HTML/CSS cleaner

instead of using ID's you should be using classes. like this

sidebar

 Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book.

then you could change it a little bit more because you have the same exact CSS for 3 & 4 so I would change those to something like this.

#smallLeft {
    float:left;
    width:236px;
    height:167px;
}

    #rtMargin {
    margin-right:5px;
}

same concept for items 2 & 5, and remember that CSS executes top to bottom and will overwrite itself if you change an element with two different selectors in different CSS documents or even on the same document it will overwrite the style, the last one will prevail (in that instance)

Something from my Comment

you should provide all the html, I don't know what is going on with your Headers, but the CSS for them looks wrong. you should apply the width to the outermost element and then 100% the elements inside that you want to be the same width. –  Malachi 1 hour ago


How do I shorten my JQuery tabs? I'm trying to clean up my JQuery code for my tabs. I have 3 different styles. How do I make the following shorter?

$(document).ready(function () {
    $('.tabs-style1 li').click(function () {
        var tab_id = $(this).attr('data-tab');
        $('.tabs-style1 li').removeClass('current');
        $('.tab-content-style1').removeClass('current');
        $(this).addClass('current');
        $("#" + tab_id).addClass('current');
    })
})

$(document).ready(function () {
    $('.tabs-style2 li').click(function () {
        var tab_id = $(this).attr('data-tab');
        $('.tabs-style2 li').removeClass('current');
        $('.tab-content-style2').removeClass('current');
        $(this).addClass('current');
        $("#" + tab_id).addClass('current');
    })
})

$(document).ready(function () {
    $('.tabs-style3 li').click(function () {
        var tab_id = $(this).attr('data-tab');
        $('.tabs-style3 li').removeClass('current');
        $('.tab-content-style3').removeClass('current');
        $(this).addClass('current');
        $("#" + tab_id).addClass('current');
    })
})

Your help will be appreciated!
http://michellecantin.ca/test/features/tabs/

The visual style of your tabs is actually independent of their functionality. That is, they all behave as tabs, regardless of how they look.

So, while you could make your code more generic by making the selector strings on the fly ($(".tab-content-" + styleName) etc.), I'd recommend something different.

I'd start by separating the classes that describe style from the ones that describe function. Then I'd use the structure of the markup to figure out the rest; not everything needs a class name.

I.e. given this markup, which is slightly changed/simplified from yours

Tab One

Tab Two

Tab Three

Tab Four

Lorem ipsum...

Duis aute...

Ut enim...

Sed do eiusmod...

The code can be shortened to

$(".tabs").each(function () {
  var container = $(this);
  container.find(".tabs-list li").on("click", function () {
    var tab = $(this),
        target = tab.data("tab");
    tab.addClass("current")
      .siblings().removeClass("current");
    container.children("#" + target).addClass("current")
      .siblings("div").removeClass("current");
  });
});

for any and all "tab areas" on the page. It'll also work if you add new styles later, or remove any of ones you have. I.e. the visual style is decoupled from the behavior. Also, you only need to change the class name in one place to re-style an entire tab area.

CSS-wise, simply scope your rules to .style1, .style2 etc. and perhaps use the direct-descendant operator, while keeping the base styling common for all styles

.tabs-style1 .tabs-list { ... }
.tabs-style1 .tabs-list li { ... }
.tabs-style1 > div { /* tab content styling */ }

Here's a demo (it's ugly - just demonstrating the functionality)


Am I using crypt and mcrypt well in this simple encryption class? These days I read a lot here on SO about password hashing and data encryption. It's a real mess, I mean, deciding what the best practice is. I need a very simple class that can be reused anywhere and that provide a decent-but-not-paranoic security level for my PHP applications (I do not handle bank data). Additionally, I want to rely as much as possible on PHP standard libs. I came up with this:

class Security {

    public static function hashPassword($plain) {
        $salt = md5(rand(0, 1023) . '@' . time()); // Random salt
        return crypt($plain, '$2a$07$' . $salt); // '$2a$07$' is the Blowfish trigger
    }

    public static function checkPassword($plain, $hash) {
        return (crypt($plain, $hash) === $hash);
    }

    public static function generateIv() {
        $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_CBC); // It's 32
        return mcrypt_create_iv($iv_size, MCRYPT_RAND);
    }

    public static function encrypt($key, $data, $iv = null, $base64 = true) {
        if (is_null($iv)) $iv = md5($key);
        $ret = mcrypt_encrypt(MCRYPT_RIJNDAEL_256, $key, $data, MCRYPT_MODE_CBC, $iv);
        return ($base64 ? base64_encode($ret) : $ret);
    }

    public static function decrypt($key, $data, $iv = null, $base64 = true) {
        if (is_null($iv)) $iv = md5($key);
        return rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, $key, $base64 ? base64_decode($data) : $data, MCRYPT_MODE_CBC, $iv), "\0");
    }

}

As you can see, I choose to hash passwords with crypt() using Blowfish hashing algorithm. The return value of hashPassword() is the salt + hash that then I store in the DB. I made this choice because crypt() is available on every server, provides a confortable way to check hash regardless of algorithm used (it's based on salt prefix) and, I read, bcrypt is a decent hashing method.

Then, for data encryption I used mcrypt() Rijndael 256 algorithm with CBC mode. As you can see, I can use encryption methods in two way. I can pass a IV (and generateIv() helps me to create one) that I will store in the DB along crypted data, or, if I don't, a basic IV is derived from key in both crypt and decrypt process.

What do you think about it? Am I missing something? Can I be finally relaxed about hashing and encryption in my PHP applications?

Your salt generation mechanism does not seem sufficiently random. You will end up with a very small set of salts.

First of all, time() is not random at all. In second place, rand(0, 1023) is not only a relatively weak random number generator, but also generates too few bits to be enough.

All this - combined with the use of the relatively unsafe md5 algorithm - means that there's a higher-than-ideal risk of salt collision.

Will all of this still cause trouble? Frankly, I don't know. This is definitely better than storing passwords in plain text, or using unsalted hashes, but it's not perfect. Generally, I'm nervous about using not-exactly-random data for security algorithms that are meant to use random data.

For a starting point, try to look at this stackoverflow question. This is what the author does for salt generation:

Use OpenSSL random pseudo bytes, if available. Using a security library to get random data for use in another security function makes me far less nervous than ordinary rand().
Try a *NIX-specific method
If not possible, then use a mix of md5, microtime(better than time since it changes more often) and even the PID.

In particular, note than PHP's md5 normally returns a hexadecimal string with a 32-byte length, but bcrypt expects a string with a length of 22, so that might cause troubles (namely, the salt will not be as secure as it could be)

Also, in your case this won't be a problem, but $2a is ambiguous (but sadly, the correct, unambiguous way might be incompatible with older PHP versions). If you can, consider using $2y instead and see if that works for you.

The PHP documentation recommends using mcrypt_enc_get_iv_size instead of mcrypt_get_iv_size and using that in combination with mcrypt_module_open (although strangely, the documentation for mcrypt_encrypt does use mcrypt_get_iv_size)

Again, you are using not-really-random data for a function that seems to expect random data (specifically, the IV when the supplied IV is "null"). This makes me nervous.


Loading images according to screen width This uses HTML5 data attributes to load the image according to the screen width. For example, for laptops. it loads only laptop-sized images. saving bandwidth and optimizing speed.

Live example

    //---------------------------Setting ViewPort Widths-----------------------//
var sSize=768; // High End Mobile Phones
var mSize=1024; // Tables like Ipad Or Something..
var lSize=1336; // Laptop 15 Inch
var xlSize=1500;  // Desktop  22 Inch And Above

//------------------------------ Grabbing Images in Images Array ------------//
var images = document.getElementsByTagName('img');

//--------------------------------Checking Current Resolution Function -----------///
function checkRes()
{
var width = document.documentElement.clientWidth; // grab the width of the screen //
var currentFlag=""; // To Check the Final Resolution

// --------------------- Paths of Img  To Grab ---------------------------//

var  xlImgPath='data-xl';
var  lImgPath='data-l';
var  mlmgPath='data-m';
var  slmgPath='data-s';

//------------------------------  Set CurrentFlag to the Current Browser Size /////////

if (width>=xlSize) {
    currentFlag="xl";
}
else if (width>=lSize   )
{
    currentFlag="l";
}
else if (width>=mSize   )
{
    currentFlag="m";

}
else if (width<=sSize   )
{
    currentFlag="s";

}

    //-------------------------------  Cycle through all the Images And Attach the Proper Src ---------//////////
    for (var i = 0; i < images.length; i++)
    {
        var thisImage=images[i];  // Grab Current Image //
        var defaultImgSource =images[i].getAttribute('src'); // grabs the Current image src (The Default Image)

//-------------------------------------- Switch Case Accoring to Current Size ------------------

switch(currentFlag){

    case 'xl':
                if (thisImage.getAttribute(xlImgPath)!==null)  // If Desktop Image has been defined..
                {

                thisImage.src=thisImage.getAttribute(xlImgPath);   // Set The path of Image to  Desktop Path
            }
            else
            {

                thisImage.src=defaultImgSource; // Else Set the Fefaut src image

            }

            break;
            case 'l':
            if (thisImage.getAttribute(lImgPath)!==null)
            {

                thisImage.src= thisImage.getAttribute(lImgPath);
            }
            else
            {

                thisImage.src=defaultImgSource;

            }
            break;

            case 'm':
            if (thisImage.getAttribute(mlmgPath)!==null)
            {
                thisImage.src=thisImage.getAttribute(mlmgPath);
            }
            else
            {

                thisImage.src=defaultImgSource;

            }
            break;

            case 's':
            if (thisImage.getAttribute(slmgPath)!==null)
            {

                thisImage.src=thisImage.getAttribute(slmgPath);
            }

            else
            {

                thisImage.src=defaultImgSource;

            }
            break;
        }

    }
};

//---------------------------------Run The Function On Page Load ---------------------
checkRes();

HTML

        <img
src="http://s30.postimg.org/rjm6iy2kh/default.jpg"
data-s="http://s4.postimg.org/4gmjg2m7h/image.jpg"
data-m="http://s4.postimg.org/4gmjg2m7h/image.jpg"
data-l="http://s4.postimg.org/4gmjg2m7h/image.jpg"
data-xl="http://s4.postimg.org/4gmjg2m7h/image.jpg" >

I have a few comments, which in no way substitute a full review.

Your indentation is inconsistent which makes your code unnecessarily hard to read.

Your comments are also inconsistent. I'm not going to prescribe you a commenting standard, but pick a style you like and stick with it.

Now we're talking about comments, the following is redundant:

var thisImage=images[i];  // Grab Current Image //

It's still clear what you're doing without the comment. It's commenting the 'what' of your code while you should only be commenting the 'why'. The 'what' is already obvious in this case, as it should be. You may be interested in reading Writing Readable Code, about readable code and what part comments play in this. Comments can be good, but they are often not necessary.

You're only using the width of the resolution. While most desktop screens use standard ratios, you seem to target mobile users. Mobile devices don't necessarily stick to standard ratios. For width your screen may be a small, but for height it could be a medium.


Match a multi line text block I want to count the occurrences of a multi line ASCII pattern within a file.

The pattern looks like this:

a1
b2
c3

The file I want to search looks like this:
(The _ represent whitespaces but I thought it's easier to understand with this notation)

_ _ _ a1
_ _ _ b2
_ _ _ c3  _ _ _ _ a1
a1_ _ _ _ _ _ _ _ b2  _ _ _ _ a1
_ _ _ _ _ _ _ _ _ c3  _ _ _ _ b2
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ c3

The desired output should be 3 in this case. I solved this with a bunch of loops and counting symbols till finding the first part of the pattern. Then checking if the second part hast the same count of numbers before to ensure the second part is underneath the first one and so on.

_ _ _ _ a1 _ _ _ a1
_ _ _ _ b2 _ _ b2 _
_ _ _ _ c3 _ _ _ c3

In this example there is only one valid pattern found since the parts of the second aren't exactly under each other.

The regarding code is as following:

import sys

def load_data(path):
    data = []
    try:
        fp = open(path, 'r')
        for f_line in fp:
            data.append(f_line.rstrip())

    finally:
            fp.close()
    return data

bug_path = 'data/bug.txt'
landscape_path = 'data/landscape.txt'

bug = load_data(bug_path)
landscape = load_data(landscape_path)

findings = [[] for x in landscape]

min_len_bug = min([len(x) for x in bug])

for cnt, l_line in enumerate(landscape):
    if len(l_line) < min_len_bug:
        continue
    for bCnt, bLine in enumerate(bug):
        findings[cnt] = [(bCnt, ind) for ind in range(len(l_line)) if l_line.startswith(bLine, ind)] + findings[cnt]

def bugInLine(line, bug_part, whitespace):
    for entry in line:
        if entry[0] == bug_part and entry[1] == whitespace:
            return True

complete_bugs_cnt = 0

for cnt, l_line in enumerate(findings):
    for found in l_line:
        if found[0] == 0 and len(findings) > (cnt + len(bug) - 1):
            check = 1
            for i in range(1, len(bug)):
                if bugInLine(findings[cnt + i], i, found[1]):
                    check = check + 1

            if check == len(bug):
                complete_bugs_cnt = complete_bugs_cnt + 1

print complete_bugs_cnt

Since this isn't the most elegant solution I'm wondering if there is a possibility to solve this problem by using some regex code and combine this with the re.findall()-method.

I'd highly appreciate any help.

data = []
    try:
        fp = open(path, 'r')
        for f_line in fp:
            data.append(f_line.rstrip())

    finally:
            fp.close()
    return data

This can be simplified to:

data = []
with open(path) as fp:
   f_line = fp.readline()
   data.append(f_line.rstrip())
return data

The with statement in Python will automatically close the file after the code is done with it. It also handles the finally exception by closing it.

findings = [[] for x in landscape]

This initialization can be skipped if the algorithm is transformed.

for cnt, l_line in enumerate(findings):
    for found in l_line:
        if found[0] == 0 and len(findings) > (cnt + len(bug) - 1):
            check = 1
            for i in range(1, len(bug)):
                if bugInLine(findings[cnt + i], i, found[1]):
                    check = check + 1

            if check == len(bug):
                complete_bugs_cnt = complete_bugs_cnt + 1

This is close to text = [''.join(arr) for arr in zip(*text[::1])] (rotate the text by -90 degrees and search) and sum([word.count(pattern_to_find) for word in text]).

The current algorithm is too complicated. Consider an algorithm which checks if a word is present in a single line. Given the line abc  c  d e    a abc the word abc occurs twice, and .count() can be used to find how many times that pattern occurs, or, a while loop with the string split on whitespace (then check if the item is equal to that item.)

However, the text is in columns instead of rows. The text can be rotated -90 degrees which turns it into the easy-to-parse row format. From there it becomes very easy to parse.

After applying these transformations, the code becomes the following (without file reading):

def find_text_column(haystack, pattern_to_find):
  text = haystack.splitlines()
  maxlen = len(max(text, key=len))
  # make every line the same length so that zip() works, as it does not work with irregular arrays
  text = [line.ljust(maxlen, ' ') for line in text]

  # rotate string by -90 degrees
  text = [''.join(arr) for arr in zip(*text[::1])]
  return sum([word.count(pattern_to_find) for word in text])


Check if dictionary keys are declared, if not leave them as true I've tried with:

dictionary.TryGetValue("MoreDetails", out bool MoreDetails);

but if the key is not present it defaults to false, and I need it to default to true. Else I have this working implementation:

Dictionary<string, bool> CheckNullsSectionsVisibility(Dictionary<string, bool> dictionary)
{
    bool Summary = true;
    bool Backlog = true;
    bool MoreDetails = true;

    if (dictionary != null)
    {
        Summary = dictionary.ContainsKey("Summary") ? dictionary["Summary"] : true;
        Backlog = dictionary.ContainsKey("Backlog") ? dictionary["Backlog"] : true;
        MoreDetails = dictionary.ContainsKey("MoreDetails") ? dictionary["MoreDetails"] : true;
    };

    return new Dictionary<string, bool> {
        { "Summary", Summary},
        { "Backlog", Backlog},
        { "MoreDetails", MoreDetails}
    };
}

Is there a better approach? I don't like to repeat the true assignment, by the way.

TryGetValue returns True or False if it was found in the dictionary. I would suggest something like below. As it's still a single lookup in the dictionary to get the value but you can also set a default value other than default if it's not found in the dictionary.

bool moreDetails;
if !(dictionary.TryGetValue("MoreDetails", out moreDetails))
{
    moreDetails = true;
}

Update to show extension method:

public static class DictionaryExtensions
{
    public static TValue TryGetWithDefaultValue<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue)
    {
        TValue value;
        if (!dictionary.TryGetValue(key, out value))
        {
            value = defaultValue;
        }
        return value;
    }
}

Then you could call it like:

var moreDetails = dictionary.TryGetWithDefaultValue("MoreDetails", true);


Find mirror image of a matrix Looking for review, good practices, optimizations, clean code tips etc.

/**
 * Flip the columns.
 *
 * Complexity:
 * O(row * col)
 *
 */
public final class Mirror {

    private Mirror () { }

    /**
     * Given a matrix create a mirror image,
     *
     * @param m     the input matrix
     * @throws      NPE if exception occurs.
     */
    public static void mirrorPatch(int[][] m) {
        // for each row.
        for (int i = 0; i < m.length; i++) {
            // for each column
            flipRow(m[i]);
        }
    }

    private static void flipRow(int[] row) {
        int length = row.length;
        // simple swap of each element.
        for (int i = 0; i < length/2; i++) {
            int x = row[i];
            row[i] = row[length -1 - i];
            row[length -1 - i] = x;
        }
    }

    public static void main(String[] args) {
        // even number of columns.
        int[][] m = { {1, 2, 3, 4} , {10, 20, 30, 40}};
        mirrorPatch(m);

        /*
         * Informally verifying that output is:
         * 4   3   2   1
         * 40  30  20  10
         *
         */
        for (int i = 0; i < m.length; i++) {
            for (int j = 0; j < m[0].length; j++) {
                System.out.print(m[i][j] + " ");
            }
            System.out.println();
        }

        System.out.println("-----------------------------------");

        // odd number of columns.

        /*
         * Informally verifying that output is:
         *  5   4   3   2   1
         *  50  40  30  20  10
         */
        int[][] m1 = { {1, 2, 3, 4, 5} , {10, 20, 30, 40, 50}};
        mirrorPatch(m1);

        for (int i = 0; i < m1.length; i++) {
            for (int j = 0; j < m1[0].length; j++) {
                System.out.print(m1[i][j] + " ");
            }
            System.out.println();
        }

        System.out.println("-----------------------------------");
    }
}

In your top=level method mirrorPatch you have:

    // for each row.
    for (int i = 0; i < m.length; i++) {
        // for each column
        flipRow(m[i]);
    }

This is unnecessarily verbose, using the iterable nature of arrays you could simply:

    for (int[] row : m) {
        flipRow(row);
    }

This is self-documenting, and all good.

In your flipRow method, you do a bit more work, and, it is pretty well structured, and readable. If you want to perhaps squeeze out some more performance, I can suggest some changes. Specifically, since you are creating the length variable, you may as well make it more useful by changing it to be last instead, which simplifies the indexing a little bit. Additionally, sometimes (when it is possible), a subtracting-loop is faster (because the condition is simpler), consider the changes as follows:

private static void flipRow(final int[] row) {
    final int last = row.length - 1;
    for (int i = last/2; i >= 0; i--) {
        int x = row[i];
        row[i] = row[last - i];
        row[last - i] = x;
    }
}


Convert binary raster file to text CSV I have a binary file input.hgt from the Shuttle Radar Topography Mission.  It contains a 3601✕3601 matrix, stored as 2-Byte big-endian integer numbers.  My code converts it to a readable text file output.csv.

How can I make it better?

Here is my code:

#include <iostream>
#include <fstream>
using namespace std;

const int SRTM_SIZE = 3601;
static int height[SRTM_SIZE][SRTM_SIZE] = {{0},{0}};

int main() {

ifstream file("/storage/emulated/0/input.hgt", ios::in | ios::binary);

if(!file) {
cout << "Error opening file!" << endl;
return -1;
}

unsigned char buffer[2];

for (int i = 0; i < SRTM_SIZE; ++i) {

for (int j = 0; j < SRTM_SIZE; ++j) {

if(!file.read(reinterpret_cast<char*>(buffer), sizeof(buffer) )) {

    cout << "Error reading file!" << endl;

return -1; }

height[i][j] = (buffer[0] << 8) | buffer[1];

} }

ofstream meinFile;

meinFile.open ("/storage/emulated/0/output.csv");

    for(int x = 0; x < SRTM_SIZE; x++)
    { // from row 1 to row SRTM size

        for(int y = 0; y < SRTM_SIZE; y++)
        {// from column 1 to SRTM_Size

        meinFile << height[x][y] << ",";

        }

        meinFile << endl;
    }

            meinFile.close();
    cout<< "Gratulations!" <<endl;
    cout<< "Your file has been converted sucessfully" <<endl;
    return 0;
}

Avoid using namespace std.
Assuming sizeof(int) is 4, the height array takes more than 40 MB of space. I understand that by modern standards it is a small change, but still a good citizen should not request more than is really needed.

I recommend a stream-like processing: read 2 bytes, convert them, and print out. This way you only need 2 bytes of storage.
The program terminates each row with a dangling comma. I don't know what the RFC says about it. Consider however

    for(int x = 0; x < SRTM_SIZE; x++) {
        for(int y = 1; y < SRTM_SIZE - 1; y++) {
            meinFile << height[x][y] << ",";
        }
        meinFile << height[x][SRTM_SIZE - 1] << std::endl;
    }

You test reading errors. Writing may also fail.
Error detection is important. Error reporting is equally important.

Can't open file - why? Is it insufficient permission? file not found? corrupted media?

Can't read file - why? Can't write file - why?

Check out strerror, perror or similar facilities.


Mixing Async and Sync in same HTTP request ASP.NET Core C# Is it bad practice to mix async and sync call in same ASP.NET core API call?

For example, in following code method CropBlackBroderOfAnImageAsync is an async method.

On the other hand SaveImageForProcessing(file, sourceFolderPath); is a sync method.

The reason I am calling SaveImageForProcessing synchronously is
that I want use the result of it to execute the code in CropBlackBroderOfAnImageAsync.

public async Task<(string sourceFolderPath, string destinationFolderPath)> CropBlackBorderOfAnImage(IFormFile file)
{
    var extension = Path.GetExtension(file.FileName);
    var newFileName = Guid.NewGuid().ToString();//Create a new Name for the file due to security reasons.
    var fileNameSource = newFileName + extension;
    var sourceFolderPath = Path.Combine(Directory.GetCurrentDirectory(), "Images\\Source", fileNameSource);

    var fileNameDestination = newFileName + "Result" + extension;
    var destinationFolderPath = Path.Combine(Directory.GetCurrentDirectory(), "Images\\Destination", fileNameDestination);

    SaveImageForProcessing(file, sourceFolderPath);

    await _imageCropBlackBroderService.CropBlackBroderOfAnImageAsync(sourceFolderPath, destinationFolderPath);

    return (sourceFolderPath, destinationFolderPath);
}

private void SaveImageForProcessing(IFormFile file, string path)
{
    using (var bits = new FileStream(path, FileMode.Create))
    {
        file.CopyTo(bits);
    }
}

Well, generally it is not a problem to call a sync method within async method.

You are actualy doing this in more then few instances (GetExtension, NewGuild, ToString, etc...)

But if you have an async method available it would be shame to not use it.
Which you have - IFormFile.CopyToAsync().

using (var bits = new FileStream(path, FileMode.Create))
{
  await file.CopyToAsync(bits);
}
await _imageCropBlackBroderService.CropBlackBroderOfAnImageAsync(sourceFolderPath, destinationFolderPath);


Is it a good way to get Count on Linq? public class BuscaDiretaBusiness
{
    PortalIOEntities portalIO = new PortalIOEntities();

    public IQueryable CarregaAlias(out int ItemsCount)
    {
        var query = from a in portalIO.DO_CadernoBuscaDiretaAlias
                    select new
                    {
                        a.CDP_ID,
                        a.CDA_Alias,
                        a.DO_CadernoDeParaBuscaDireta.CDP_NomeCaderno
                    };
        ItemsCount = query.Count();
        return query;
    }
}

Will it make two selects on DataBase?

That method will only hit the DB once, with a query like SELECT COUNT(*) …. But if you try to enumerate the resulting IQueryable in any way (like calling ToArray() on it or using it in a foreach), it will make another DB query.

You should probably only return the IQueryable. If the consumer needs all the items, it will call ToArray() or similar, which will make one DB query and then look at the Length of the result (which of course won't make another query). If the consumer needs only part of the result (e.g. it will call Take(100)) and it also needs the total count, it will call Count() itself. This will mean there will be two queries, but I think it can't be done differently.

Also, you probably should not be returning a collection of an anonymous type from a method, because it's hard to use.


Password Trainer v1.0 - Helping you learn a new password There are two reasons why many people choose weak passwords over stronger, more complicated ones:

They don't know that their passwords are insecure
They don't want to change to a better password because they are generally more complicated and harder to remember

So, I came up with this program: a password trainer! It helps you learn a new password, which should hopefully make point no. 2 more appealing.

While I am not new to the Python language, this my first attempt at correct styling according to PEP 0008. I have been focousing on the correct use of docstrings and commenting.

Code:

"""
Password Trainer v1.0
Copyright (c) 2016 angussidney
Released under the MIT license

This program helps you remember long, complicated, (and most importantly) secure passwords.
"""

import os

PASSWORD = "password123" # Insert your password here
MAXIMUM_TRIES = 3 # The maximum number of attempts before the program stops annoying the user.

def main():
    """Prompts the user for their password and provides feedback."""
    tries = 0
    while tries < MAXIMUM_TRIES:
        password_attempt = raw_input("Please type your password: ")
        print
        if password_attempt == PASSWORD:
            print "You entered your password correctly."
            break # Skips directly to end of session
        else:
            print "Your attempt was incorrect.\n"
            print_differences(password_attempt)
            raw_input("\nPress any key to continue...")
            tries += 1
            clear_screen()
    print "\nPassword training is complete for this session."
    raw_input("Press any key to continue...")

def clear_screen():
    """Clears the screen for a new password attempt."""
    os.system("cls" if os.name=="nt" else "clear")

def print_differences(attempt):
    """Prints the differences between the password attempt and the real password.
    Raises an error if the attempt is of the wrong length.

    Keyword arguments:
    attempt -- the password attempt

    Example output:
    Correct password: password123

    Your attempt:     passwood124
    Errors:                 x   x
    Corrections:            r   3
    """
    if len(attempt) != len(PASSWORD):
        print "Your password attempt was of the wrong length."
        print "Your attempt should have been %s characters long." % str(len(PASSWORD))
        print "No password accuracy breakdown available."
    else:
        print "Correct password: %s\n" % PASSWORD
        print "Your attempt:     %s" % attempt
        print "Errors:           ",
        for letter in range(len(PASSWORD)):
            if attempt[letter] == PASSWORD[letter]:
                print "\b ", # \b backspace character must be used to remove automatically added space
            else:
                print "\bx",
        print
        print "Corrections:      ",
        for letter in range(len(PASSWORD)):
            if attempt[letter] == PASSWORD[letter]:
                print "\b ",
            else:
                print "\b%s" % PASSWORD[letter],
        print

if __name__ == "__main__":
    main()

Notes:

Currently, the program simply returns an error if the length of the password attempt is different to that of the actual password. This will be changed in v2.0.
The program only works when run in the console. It does not work in IDLE (because the clear screen function and the backspace characters won't work)

What needs reviewing:

I would be greatful if you could review the following items:

Is my code following the PEP 0008 standard? Especially the comments and docstrings?
Is there a better way to print the corrections and errors, without using backspace characters (\b)?

password_attempt = raw_input(...)

When dealing with passwords, you want to try and minimize the amount of information any bypasser can see on the screen.

Python offers the getpass module to ease such inputs:

import getpass

password_attempt = getpass.getpass(...)

As such, you should also avoid printing plain passwords and attempts.


Storing data into an SQL table using multiple threads and queues I have a process running in a separate thread which reads data over Ethernet and raises an event for each read. Then, this data is processed over a tree of classes. At the top of my class tree, I want to store the processed data to SQL tables. This read rate can be rather fast and often at times but on average its slower than my write rate to SQL. This is why I want a queue. Also I want my window not to freeze, so I want a separate thread.

The code below works but I'm fairly inexperienced in multi threading. Is this a good and robust way to accomplish my task?

Private addLogQueue As New Queue
Private dequeueing As Boolean = False
Public Sub addLogHandler(ByVal machineName As String, ByVal LogTime As Date, ByVal EventLog As String, ByVal EventValue As String)
    addLogQueue.Enqueue("INSERT INTO ... some SQL CODE")
    Dim Thread1 As New System.Threading.Thread(AddressOf addLog)
    Thread1.Start
End Sub
Private Sub addLog()
    If Not dequeueing Then
        dequeueing = True
        While addLogQueue.Count <> 0
            Try
                SQLCon.Open()
                SQLCmd = New SqlCommand(addLogQueue.Dequeue(), SQLCon)
                SQLCmd.ExecuteNonQuery()
                SQLCon.Close()
            Catch ex As Exception
                MsgBox(ex.Message)
                If SQLCon.State = ConnectionState.Open Then
                    SQLCon.Close()
                End If
            End Try
        End While
        dequeueing = False
    End If
End Sub

After thinking a bit I realize I might not even need a queue or the dequeueing boolean, can I just start a new thread for every write maybe. I added the queue before I added the multi thread because at times I had to write before the SQLcon is even closed. Since I've added a thread, should I even keep the queue?

Single-Threading Style:

Managing resources:

You should be using using to manage your connection. It completely eliminates that... unsightly Try-Catch-Block. Compare following snippet from msdn with a similarly stripped version of your code:

Using connection As New SqlConnection(connectionString)
    connection.Open()
    ' Do work here; connection closed on following line.
End Using

Try
    SQLCon.Open()
    ' Do work here; connection closed on following line.
    SQLCon.Close()
Catch ex As Exception
    MsgBox(ex.Message)
    If SQLCon.State = ConnectionState.Open Then
        SQLCon.Close()
    End If
End Try

Naming

While SQLCon and SQLCmd are ... conventional names, they are a little short and have the air of systems-hungarian... I'd prefer connection and command.

Also Thread1 is really non-speaking, InsertingThread may be the better option.

Whitespace

I like to keep separate Subs clearly outlined by an empty line before starting with [Modifier] Sub ...

Comments and documentation

You may have removed it, but ... this code is extremely undercommented and underdocumented. A comment here and there can help a little wit easing the reading. Then again this code is rather simple and obvious by the names...

Multi-Threading Style:

First off, you currently only allow a single Thread to run... well almost, because of possible interleaving and nonatomic comparison operations.

Consider the following:

Thread 1                   Thread 2
- If Not deququeing
                           - If Not dequeuing
                               - dequeueing = True
    - dequeueing = True

Suddenly you have 2 threads running and that when you could've had everything easy.

The queue docs mention ConcurrentQueue(Of T) when you "need to access the queue across multiple threads"
This makes the failed attempt at synchronizing with a shared boolean unnecessary and overall improves the code quality.

End result:

After applying my critiques I end up with following code:

Private addLogQueue As New ConcurrentQueue(Of String)

Public Sub addLogHandler(ByVal machineName As String, ByVal LogTime As Date, ByVal EventLog As String, ByVal EventValue As String)
    addLogQueue.Enqueue("INSERT INTO ... some SQL CODE")
    Dim InsertingThread As New System.Threading.Thread(AddressOf addLog)
    InsertingThread.Start
End Sub

Private Sub addLog()
   Using (connection As New SqlConnection(connstring))
        connection.Open()
        Dim statement As String
        While addLogQueue.TryDequeue(out statement)
            command = New SqlCommand(statement, connection)
            command.ExecuteNonQuery()
        End While
    End Using
End Sub


Project Euler #3 in C# - largest prime factor I just started doing the Project Euler challenges and right now I'm on Challenge #3 - The largest prime factor of 600851475143.

I wrote some code and tried to optimize it based on some other Stack Overflow users' advice.

What I got is this code which does its job, but I think it takes longer than it should - calculating high numbers like 600851475143 takes forever!

Am I in the right way? How could I optimize it?

using System;

namespace ProjectEuler_LargestPrimeFactor
{
    class Program
    {
        static long numberToFactor = 600851475143;

        static void Main(string[] args)
        {
            Console.WriteLine(LargestPrimeFactorOf(numberToFactor) + " is the largest prime factor of " + numberToFactor);
            Console.ReadKey();
        }

        static long LargestPrimeFactorOf(long n)
        {
            long lastPrimeFactor = 0;

            for (long i = 2; i < n; i++)
            {
                if (isPrime(i) && n % i == 0)
                {
                    lastPrimeFactor = i;
                    Console.WriteLine(i + " is a prime factor of " + n);
                }
            }

            return lastPrimeFactor;
        }

        static bool isPrime(long n)
        {
            if (n == 2) return true;
            if ((n > 2 && n % 2 == 0) || n == 1) return false;

            for (long i = 2; i <= Math.Floor(Math.Sqrt(n)); ++i)
            {
                if (n % i == 0) return false;
            }

            return true;
        }
    }
}

Here's my solution to this problem

    static void Main(string[] args)
    {
        long num = 600851475143;
        int count = 3;
        Stopwatch sw = Stopwatch.StartNew();
        while (num > 1)
        {
            if (num%count == 0)
            {
                num /= count;
            }
            else
            {
                count += 2;
            }
        }
        sw.Stop();
        Console.WriteLine("The largest prime factor of the number {0} is {1} ", 600851475143, count);
        Console.WriteLine("Time to calculate in milliseconds : {0}", sw.ElapsedMilliseconds);
        Console.ReadKey();
    }

First of all most of the exercises you will find on Project Euler will require some math formula or concept in order to achieve the highest possible performance. It's not pure programming and thus not the best place to practice it in my opinion (ofc assuming you don't know all the formulas) you will probably learn more math than programming in the process which is a good thing if that's what you are looking for. Long story short look for formula before you go into the programming.

Back to your actual code :

Why is your code working slow ?
 There are few points here that I want to make first method calls are slower than just a bunch of code in a single method again some problems you will find later in Project Euler will be pretty easy to solve but hard to optimize so the usual C# code has everything separated in methods and classes because the performance loss is not that big in a normal project however here this might be your only bottleneck in some cases. You should always think in perspective what if you had even bigger number ? Would your method still work fast enough ? A working program doesn't mean that it's a good program.

Next you start thinking what can you improve and why exactly you are doing certain things. Do you really need to know every single number that is prime ?

In my solution there is simple prime factorization starting from the smallest prime we keep increasing the current number we have until we get a number that we can divide only by itself i.e the  largest prime factor. 100,000 iterations run for about 3000-3200 ms on my machine.


DropDown tree with search I fetch a bunch of categories for a DropDown tree, and have to implement search in this dropdown. If there is a match in one of the elements, it should also get all parents to display in the dropdown hierarchy. The number of childnodes is unknown, and can be changed at any given time (example has only 3 levels but can be 5 also).

The search applies on every ScrapCategory description.

I get the expected result but I would love to see this in \$O(N)\$.

public sealed class ScrapCategory
{
    public int Id { get; set; }

    public bool IsActive { get; set; }

    public string Description { get; set; }

    public int Level { get; set; }

    public int ParentId { get; set; }
}

public ScrapCategory[] Filter(ScrapCategory[] categories, string searchString)
{
    var result = new List<ScrapCategory>();

    foreach (ScrapCategory category in categories)
    {
        if (category.Description.IndexOf(searchString, StringComparison.OrdinalIgnoreCase) >= 0)
            result.Add(category);

        if (category.ParentId > 0)
        {
            int parentId = category.ParentId;

            while (parentId > 0)
            {
                var parent = categories.Where(x => x.Id == parentId)?.First();
                if (!result.Contains(parent))
                    result.Add(parent);
                parentId = parent.ParentId;
            }
        }
    }

    return result.OrderBy(x => x.Level).ToArray();
}

var result = new List<ScrapCategory>();
    ...
                if (!result.Contains(parent))

There's one performance problem. Use a data structure which gives fast Contains checks: typically HashSet<>.

                var parent = categories.Where(x => x.Id == parentId)?.First();

There's another one. I assume that IDs are unique, in which case lookup = categories.ToDictionary(category => category.Id) will give you a fast hash map from ID to category.

        if (category.Description.IndexOf(searchString, StringComparison.OrdinalIgnoreCase) >= 0)
            result.Add(category);

        if (category.ParentId > 0)
        {
            int parentId = category.ParentId;

            while (parentId > 0)
            {
                ...
            }
        }

Is this definitely correct? It's not supposed to be

        if (category.Description.IndexOf(searchString, StringComparison.OrdinalIgnoreCase) >= 0)
        {
            result.Add(category);

            if (category.ParentId > 0)
            {
                ...
            }
        }

?

Either way, the if (category.ParentId > 0) is pointless. The test is repeated by the while loop, and a single assignment to a local variable isn't going to be a performance bottleneck.

                if (!result.Contains(parent))
                    result.Add(parent);

Note that with the suggestion of making result a HashSet it becomes preferable to just call Add, which returns a bool to tell you whether it changed anything. If it didn't, you can break out of the loop, because you know that all of the further ancestors have already been added.

The effect of these changes (except the one I'm unsure about) is

public ScrapCategory[] Filter(ScrapCategory[] categories, string searchString)
{
    var lookup = categories.ToDictionary(category => category.Id);
    var result = new HashSet<ScrapCategory>();

    foreach (ScrapCategory category in categories)
    {
        if (category.Description.IndexOf(searchString, StringComparison.OrdinalIgnoreCase) >= 0)
            result.Add(category);

        int parentId = category.ParentId;
        while (parentId > 0)
        {
            var parent = lookup[parentId];
            if (!result.Add(parent))
                break;
            parentId = parent.ParentId;
        }
    }

    return result.OrderBy(x => x.Level).ToArray();
}


The easiest (but not computationally most efficient) way to solve this challenge is to use a brute-force algorithm, like you did. However you can write it a lot more succinctly when using more of the tools available in the Python standard library.

Use a generator expression to get all slices:

s = "..."
n = 13
slices = (s[i:i+n] for i in range(len(s) - n))
next(slices)
# '7316717653133'

map this string to an iterable of integers:

slices = (s[i:i+n] for i in range(len(s) - n))
list(map(int, next(slices)))
# [7, 3, 1, 6, 7, 1, 7, 6, 5, 3, 1, 3, 3]

Use functools.reduce and operator.mul to get the product of this iterable:

from functools import reduce
from operator import mul

slices = (s[i:i+n] for i in range(len(s) - n))
reduce(mul, map(int, next(slices)))
# 5000940

Use max to...get the maximum of those:

slices = (s[i:i+n] for i in range(len(s) - n))
max(reduce(mul, map(int, slice)) for slice in slices)

Finally, wrap the code in functions and the calling code in a if __name__ == "__main__": guard:

from functools import reduce
from operator import mul

def prod(x):
    return reduce(mul, x)

def maximum_product(s, n):
    return max(prod(map(int, s[i:i+n])) for i in range(len(s) - n))

if __name__ == "__main__":
    s = "..."
    n = 13
    print(max_product(s, n))