אהלן חברים.

כתבתי תוכנית קטנה שמחפשת מחרוזרת בתוך קבצים, ורציתי לשמוע חוות דעת לגבי סגנון הכתיבה שלי, אני מנסה לשפר את ההרגלי כתיבה וסגנון הכתיבה.

לא מזמן נכנסתי לשפות מונחות עצמים, לכן אבקש דגשים בכל המובנים הנ"ל של הקוד. לדוגמה האם אני עושה שימוש נכון בclass שעשיתי ? האם לא היה כדאי לעשות אותו סטטי וכו'.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;
using System.Diagnostics;
using System.Collections;

namespace Search_Files
{
  class TestProgram
  {
    static void Main(string[] args)
    {
      FileSearch.DirectoryPath = @"D:\My Documents\Lyrics\";
      FileSearch.Pattern = "*.*";
      FileSearch.IncludeSubDirectories = false;
      StringSearch.CaseSensitive = false;

      string SearchFor = Console.ReadLine();

      try
      { 
        string[] Files = FileSearch.GetFiles();
        Stopwatch Test = new Stopwatch();
        Test.Start();
        foreach (string sPath in Files)
        {
          StreamReader Stream = new StreamReader(sPath);
          string sFile = Stream.ReadToEnd();  //Still need to catch the exception that might be thrown here.

          int[] Results;
          if ((Results = StringSearch.Find(SearchFor, sFile)).Length != 0)
          {
            Console.WriteLine("Found " + Results.Length + " match(es) for the search in the file:\n" + sPath);
            Console.WriteLine("Matches in the following locations: ");
            foreach (int loc in Results)
            {
              Console.Write(loc + "\n");
            }
          }
        }
        Test.Stop();
        Console.WriteLine("\n\nSearch took: " + Test.Elapsed.TotalMilliseconds + " miliseconds\n\n");
      }
      catch (SystemException ex)
      {
        Console.WriteLine("\n Exception cought !\n" + ex.Message);
      }

      Console.ReadLine();  
    }
  }
  static class StringSearch
  {
    private static StringComparison CompareOptions = StringComparison.OrdinalIgnoreCase;

    public static bool CaseSensitive
    {
      get
      {
        if (CompareOptions == StringComparison.Ordinal)
        {
          return true;
        }
        else
        {
          return false;
        }
      }
      set
      {
        if (value == true)
        {
          CompareOptions = StringComparison.Ordinal;
        }
        else
        {
          CompareOptions = StringComparison.OrdinalIgnoreCase;
        }
      }
    }

    public static int[] Find(string substr, string str)
    {
      return Find(substr, str, 0);
    }
    public static int[] Find(string substr, string str, int loc)
    {
      ArrayList SearchResults = new ArrayList();
      for (int nextloc = loc; (nextloc = str.IndexOf(substr, nextloc, CompareOptions)) >= loc; nextloc++)
      {
        SearchResults.Add(nextloc);
        nextloc = str.IndexOf(substr, nextloc, CompareOptions);
      }
      return (int[])SearchResults.ToArray(typeof(int));
    }
  }
  static class FileSearch
  {
    public static string DirectoryPath="C:\\";
    public static string Pattern="*.*";
    public static bool IncludeSubDirectories = false;
    //Must catch exceptions from this method.
    public static string[] GetFiles()
    {
      if (IncludeSubDirectories)
      {
        return Directory.GetFiles(DirectoryPath, Pattern,SearchOption.AllDirectories); //May throw an exception.
      }
      else
      {
        return Directory.GetFiles(DirectoryPath, Pattern,SearchOption.TopDirectoryOnly); //May throw an exception.
      }
    }
  }
}

כרגע אני מממש את התוכנית בסביבה של הקונסול, ולרוב אפשרויות החיפוש אין גישה למשתמש. בשלב מסויים אני רוצה להעביר את התוכנית לסביבה של Windows Forms.

באופן כללי, עדיף להימנע משימוש ב-static בכלל. אין באמת סיבה שהמחלקות FileSearch ו-StringSearch יהיו סטטיות, והאיברים שלהן גם כן לא צריכים להיות סטטיים. חוץ מזה, לא נהוג לחשוף איברים של מחלקה כ-public (נהוג לשים אותם כ-private/protected ולחשוף באמצעות properties, כמו שעשית עם CaseSensitive).

עוד הערת אגב קטנה, לגבי ביטויים בוליאנים. קטע קוד כזה:

if (expression) {
 return true;
} else {
 return false;
}

אפשר פשוט להחליף ב:

return expression;

והקוד יהיה הרבה יותר קצר וברור באותה מידה. באותו אופן, בבדיקה:

if (value == true)

יש יתירות - אפשר פשוט לעשות:

if (value)

ועוד הערה לגבי stream - המחלקה הזו היא IDisposable, מה שאומר שהיא משתמשת במשאבים שלא משתחררים אוטומטית (במקרה הזה, קובץ), ולכן חייבים לדאוג לשחרר אותה ידנית. יש שתי דרכים לעשות את זה - באמצעות using או באמצעות קריאה ישירה ל-Dispose בסיום העבודה איתה.

http://msdn.microsoft.com/en-us/library/yh598w02.aspx

תודה רבה על כל ההערות.

למה באמת נהוג לא לחשוף את המשתנים כpublic ? זה לא סתם מיותר לעשות property עבור השמה פשוטה ? מילא אם צריך לבדוק את תקינות הנתונים.

מה הסיבה שבגללה עדיף להמנע משימוש בstatic ? אם המחלקות FileSearch ו StringSearch לא יהיו סטטיות אצתרך ליצור אובייקט מהטיפוס שלהם, שניראה כיביכול מיותר כיוון שאני לא צריך כמה אובייקטים כאלה.

האם לא מומלץ להשתמש גם במשתנים (או properties) סטטים, כדי לשלוט על אופן ביצוע הפונקציות הרצויות ע"י המחלקה ? לדוגמה האם בחיפוש להתעלם מגודל האות\ האם לחפש בתתי תיקיות וכו'.

עריכה:

איפה אני רואה שהמחלקה StreanReader מממשת את הממשק iDisposable ? בClass Viewer בVS רשום רק:

public class StreamReader : System.IO.TextReader
  Member of System.IO

Summary:
Implements a System.IO.TextReader that reads characters from a byte stream in a particular encoding.

Attributes:
[System.Runtime.InteropServices.ComVisibleAttribute(true)]

ועכשיו שאני רואה, אני בעצם יכול להשתמש בTextReader בתוכנית שלי במקום StreamReader, לא ?

למה באמת נהוג לא לחשוף את המשתנים כpublic ? זה לא סתם מיותר לעשות property עבור השמה פשוטה ? מילא אם צריך לבדוק את תקינות הנתונים.

עדיין של נוהג. לא חובה, אבל זה פשוט יפה יותר. ככה בעתיד תוכל להוסיף בדיקות תקינות קלט בקלות.

מה הסיבה שבגללה עדיף להמנע משימוש בstatic ? אם המחלקות FileSearch ו StringSearch לא יהיו סטטיות אצתרך ליצור אובייקט מהטיפוס שלהם, שניראה כיביכול מיותר כיוון שאני לא צריך כמה אובייקטים כאלה.

ומי אמר שבעתיד לא תצטרך? יצירת מופע של כל מחלקה הוא בדיוק שורה אחת. כשהכל סטטי, זה כאילו בכלל לא השתמשת במחלקות, ואז מה הרווחת כאן בכלל?

האם לא מומלץ להשתמש גם במשתנים (או properties) סטטים, כדי לשלוט על אופן ביצוע הפונקציות הרצויות ע"י המחלקה ? לדוגמה האם בחיפוש להתעלם מגודל האות\ האם לחפש בתתי תיקיות וכו'.

מומלץ? תעשה מה שאתה רוצה. אם אתה רוצה שיהיה אפשר לשלוט על התכונות האלו, אז תוסיף אותן.

אם תלך להגדרה של TextReader תראה שהוא כן Disposable. חוץ מזה, תראה שבכל דוגמת שימוש ב-StreamReader משתמשים ב-using.

אתה לא יכול להשתמש ישירות ב-TextReader כי הוא אבסטרקטי.

ומי אמר שבעתיד לא תצטרך? יצירת מופע של כל מחלקה הוא בדיוק שורה אחת. כשהכל סטטי, זה כאילו בכלל לא השתמשת במחלקות, ואז מה הרווחת כאן בכלל?

אז זהו, ששוב אני אומר שאני חדש בכל התחום הזה של שפה מונחת עצמים, אני רגיל כניראה לחשיבה של שפת C. אשמח אם תוכל להסביר לי קצת יותר לעומק\ או להפנות אותי למקור אחר- באילו מקרים באמת עדיף לבנות מחלקה סטטית ובאילו מקרים עדיף ליצור מופע של מחלקה.

בעקרון, ברירת המחדל שלך לא צריכה להיות static. למעשה, בתור מישהו שרק נחשף לעולם של תכנות מונחה עצמים, אתה לא צריך אפילו להכיר את המילה הזו עדיין. כשכל המחלקה היא סטטית, זה שקול לכך שיש רק מופע אחד שלה שאתה יכול להשתמש בו. זה בעייתי כשאתה רוצה לדוגמה להשתמש בו בכמה מקומות שונים במקביל.

אני אתן לך דוגמה - אתה כותב את התכנית שלך, אבל רוצה לאפשר לשני חיפושים או יותר לרוץ במקביל (נניח, אם יש לך תצוגה גרפית אז אתה יכול לעשות את זה עם טאבים). שני החיפושים למעשה יתנגשו - על מנת להפעיל אחד מהם תצטרך לחכות שהאחר יסתיים, אחרת יכול להיות שיעברו הפרמטרים הלא נכונים לחיפוש. אפילו אם אתה לא צריך שממש יעבדו במקביל, כלומר כל פעם חיפוש אחד יהיה "פעיל", עדיין יהיו לך בעיות - יצרת חיפוש אחד, שינית לו הגדרות, ועכשיו אתה רוצה ליצור הגדרות לחיפוש האחר - אי אפשר, כי אתה חייב לדרוך על ההגדרות הקודמות.

מתי כן להשתמש ב-static? במקרים בודדים מאוד. כרגע אני יכול לחשוב על שלושה מקרים, אבל יש עוד:

א. להשתמש בשדות סטטיים כאשר אתה רוצה שיהיה מידע מסוים שיהיה משותף לכל המופעים של המחלקה. דוגמה נפוצה לכך היא reference counting, שזו דרך לזכור כמה מופעים של המחלקה יש לך.

ב. להשתמש במתודות סטטיות כאשר המתודה לא משתמשת בשום מידע שמור. לדוגמה, נניח שיש לך מחלקה שממירה מספרים למחרוזות ולהיפך. המחלקה הזו מכילה רק פונקציות והיא לא צריכה לשמור שום מידע, ולכן אין שום משמעות למופעים שלה.

ג. המקרה היחודי שבו אתה רוצה לחייב שימוש במופע אחד בלבד של המחלקה. גם במקרה כזה, אגב, לא עושים את כל המחלקה סטטית, אלא עושים מופע אחד ויחיד שלה שהגישה אליו היא סטטית. זו תבנית עיצוב בשם Singleton, קרא עליה בויקיפדיה.

תודה רבה שניצל.

הבנתי את מה שניסית להעביר לי - בתיכנות עדיף לכתוב מחלקות כלליות כמה שיותר כך שיהיה אפשר להשתמש בהם גם התוכניות אחרות ולכן הייתי צריך לחשוב לעתיד ולהגיד - ואללה אולי אני ארצה לבנות ממשק "טפסי חלונות" (נשמע על הפנים בעברית) ואולי ארצה מתישהו אפשרות להריץ יותר מחיפוש אחד בו זמנית.

אני צודק ?

בדיוק. ומופעים שונים של אותה מחלקה לא צריכים להיות תלויים זה בזה.